From 59b43fa9b13b7b9dd27ce9cd52502bde1d1ce7d4 Mon Sep 17 00:00:00 2001 From: whoisasx Date: Thu, 11 Jun 2026 17:28:24 +0530 Subject: [PATCH 1/6] feat: add notifications v1 --- backend/internal/daemon/daemon.go | 20 ++- backend/internal/daemon/lifecycle_wiring.go | 8 +- backend/internal/daemon/wiring_test.go | 2 +- backend/internal/domain/notification.go | 86 +++++++++++ backend/internal/httpd/api.go | 36 +++-- backend/internal/httpd/apispec/openapi.yaml | 124 +++++++++++++++ .../internal/httpd/apispec/specgen/build.go | 23 +++ backend/internal/httpd/controllers/dto.go | 33 ++++ .../httpd/controllers/notifications.go | 102 ++++++++++++ .../httpd/controllers/notifications_test.go | 97 ++++++++++++ backend/internal/lifecycle/manager.go | 104 +++++++++---- backend/internal/lifecycle/manager_test.go | 138 +++++++++++++++++ backend/internal/lifecycle/reactions.go | 68 +++++++- backend/internal/ports/notifications.go | 27 ++++ .../service/notification/dispatcher.go | 32 ++++ .../internal/service/notification/enrich.go | 90 +++++++++++ .../internal/service/notification/service.go | 119 ++++++++++++++ .../service/notification/service_test.go | 127 +++++++++++++++ .../internal/service/notification/store.go | 14 ++ .../internal/service/notification/types.go | 41 +++++ backend/internal/storage/sqlite/gen/models.go | 12 ++ .../storage/sqlite/gen/notifications.sql.go | 146 ++++++++++++++++++ .../sqlite/migrations/0011_notifications.sql | 43 ++++++ .../storage/sqlite/queries/notifications.sql | 19 +++ .../sqlite/store/notification_store.go | 96 ++++++++++++ .../sqlite/store/notification_store_test.go | 90 +++++++++++ backend/sqlc.yaml | 16 ++ frontend/src/api/schema.ts | 95 ++++++++++++ 28 files changed, 1755 insertions(+), 53 deletions(-) create mode 100644 backend/internal/domain/notification.go create mode 100644 backend/internal/httpd/controllers/notifications.go create mode 100644 backend/internal/httpd/controllers/notifications_test.go create mode 100644 backend/internal/ports/notifications.go create mode 100644 backend/internal/service/notification/dispatcher.go create mode 100644 backend/internal/service/notification/enrich.go create mode 100644 backend/internal/service/notification/service.go create mode 100644 backend/internal/service/notification/service_test.go create mode 100644 backend/internal/service/notification/store.go create mode 100644 backend/internal/service/notification/types.go create mode 100644 backend/internal/storage/sqlite/gen/notifications.sql.go create mode 100644 backend/internal/storage/sqlite/migrations/0011_notifications.sql create mode 100644 backend/internal/storage/sqlite/queries/notifications.sql create mode 100644 backend/internal/storage/sqlite/store/notification_store.go create mode 100644 backend/internal/storage/sqlite/store/notification_store_test.go diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index 260ccd55..58dae216 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -15,6 +15,7 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/config" "github.com/aoagents/agent-orchestrator/backend/internal/httpd" "github.com/aoagents/agent-orchestrator/backend/internal/runfile" + notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification" projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project" reviewsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/review" "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" @@ -82,11 +83,15 @@ func Run() error { // Built before the Lifecycle Manager so the LCM can use it for SCM-driven // agent nudges (CI failure, review feedback, merge conflict). messenger := newSessionMessenger(store, runtimeAdapter, log) + notifier := notificationsvc.New(notificationsvc.Deps{ + Store: store, + Dispatcher: notificationsvc.NewDashboardDispatcher(nil), + }) // Bring up the Lifecycle Manager and the reaper first: it makes the session // lifecycle write path live (reducer write -> store -> DB trigger -> // change_log -> poller -> broadcaster) and gives startSession the shared LCM. - lcStack := startLifecycle(ctx, store, runtimeAdapter, messenger, log) + lcStack := startLifecycle(ctx, store, runtimeAdapter, messenger, notifier, log) lcStack.scmDone = startSCMObserver(ctx, store, lcStack.LCM, log) // Wire the controller-facing session service over the same store + LCM, the @@ -104,12 +109,13 @@ func Run() error { } srv, err := httpd.NewWithDeps(cfg, log, termMgr, httpd.APIDeps{ - Projects: projectsvc.New(store), - Sessions: sessionSvc, - Reviews: reviewsvc.NewInMemory(), - CDC: store, - Events: cdcPipe.Broadcaster, - Activity: lcStack.LCM, + Projects: projectsvc.New(store), + Sessions: sessionSvc, + Reviews: reviewsvc.NewInMemory(), + Notifications: notifier, + CDC: store, + Events: cdcPipe.Broadcaster, + Activity: lcStack.LCM, }) if err != nil { stop() diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index b4d40905..c57e5062 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -20,6 +20,10 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" ) +type notificationSink interface { + Notify(context.Context, ports.NotificationIntent) error +} + // lifecycleStack owns the runtime reaper goroutine started with the lifecycle // reducer. The reducer itself is only used for wiring observations into storage. type lifecycleStack struct { @@ -35,8 +39,8 @@ type lifecycleStack struct { // reaper. The goroutine stops when ctx is cancelled; Stop waits for it to drain. // The messenger is the per-daemon agent messenger the LCM uses to nudge agents // in response to SCM observations (CI failure, review feedback, merge conflict). -func startLifecycle(ctx context.Context, store *sqlite.Store, runtime ports.Runtime, messenger ports.AgentMessenger, logger *slog.Logger) *lifecycleStack { - lcm := lifecycle.New(store, messenger) +func startLifecycle(ctx context.Context, store *sqlite.Store, runtime ports.Runtime, messenger ports.AgentMessenger, notifier notificationSink, logger *slog.Logger) *lifecycleStack { + lcm := lifecycle.New(store, messenger, lifecycle.WithNotificationSink(notifier)) rp := reaper.New(lcm, store, runtime, reaper.Config{Logger: logger}) return &lifecycleStack{LCM: lcm, reaperDone: rp.Start(ctx)} } diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index 0e4815d0..df194ad6 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -315,7 +315,7 @@ func TestWiring_StartLifecycleThreadsMessengerIntoLCM(t *testing.T) { log := slog.New(slog.NewTextHandler(io.Discard, nil)) messenger := &captureMessenger{} - stack := startLifecycle(ctx, store, zellij.New(zellij.Options{}), messenger, log) + stack := startLifecycle(ctx, store, zellij.New(zellij.Options{}), messenger, nil, log) t.Cleanup(stack.Stop) t.Cleanup(cancel) diff --git a/backend/internal/domain/notification.go b/backend/internal/domain/notification.go new file mode 100644 index 00000000..ed5a8aa4 --- /dev/null +++ b/backend/internal/domain/notification.go @@ -0,0 +1,86 @@ +package domain + +import ( + "errors" + "time" +) + +// NotificationType identifies a user-facing notification kind persisted for the dashboard. +type NotificationType string + +const ( + // NotificationNeedsInput means an agent session is waiting for user input. + NotificationNeedsInput NotificationType = "needs_input" + // NotificationReadyToMerge means a PR has no known merge blockers. + NotificationReadyToMerge NotificationType = "ready_to_merge" + // NotificationPRMerged means a tracked PR was merged. + NotificationPRMerged NotificationType = "pr_merged" + // NotificationPRClosedUnmerged means a tracked PR closed without merging. + NotificationPRClosedUnmerged NotificationType = "pr_closed_unmerged" +) + +// Valid reports whether t is one of the v1 notification kinds. +func (t NotificationType) Valid() bool { + switch t { + case NotificationNeedsInput, NotificationReadyToMerge, NotificationPRMerged, NotificationPRClosedUnmerged: + return true + default: + return false + } +} + +// NotificationStatus is the read state for a stored notification. +type NotificationStatus string + +const ( + // NotificationUnread marks a notification that has not been acknowledged. + NotificationUnread NotificationStatus = "unread" + // NotificationRead marks a notification that has been acknowledged. + NotificationRead NotificationStatus = "read" +) + +// Valid reports whether s is a supported notification read state. +func (s NotificationStatus) Valid() bool { + switch s { + case NotificationUnread, NotificationRead: + return true + default: + return false + } +} + +// NotificationRecord is the durable notification persistence shape. +type NotificationRecord struct { + ID string + SessionID SessionID + ProjectID ProjectID + PRURL string + Type NotificationType + Title string + Body string + Status NotificationStatus + CreatedAt time.Time +} + +var ( + // ErrInvalidNotificationType reports an unknown notification type. + ErrInvalidNotificationType = errors.New("invalid notification type") + // ErrInvalidNotificationStatus reports an unknown notification status. + ErrInvalidNotificationStatus = errors.New("invalid notification status") + // ErrInvalidNotificationRecord reports a missing required notification field. + ErrInvalidNotificationRecord = errors.New("invalid notification record") +) + +// Validate checks the required fields and enum values for a stored notification. +func (r NotificationRecord) Validate() error { + if r.SessionID == "" || r.ProjectID == "" || r.Title == "" || r.CreatedAt.IsZero() { + return ErrInvalidNotificationRecord + } + if !r.Type.Valid() { + return ErrInvalidNotificationType + } + if !r.Status.Valid() { + return ErrInvalidNotificationStatus + } + return nil +} diff --git a/backend/internal/httpd/api.go b/backend/internal/httpd/api.go index d3969e8b..19b6df44 100644 --- a/backend/internal/httpd/api.go +++ b/backend/internal/httpd/api.go @@ -18,24 +18,26 @@ import ( // APIDeps bundles every service the API layer's controllers depend on. type APIDeps struct { - Projects projectsvc.Manager - Sessions controllers.SessionService - Activity controllers.ActivityRecorder - PRs prsvc.ActionManager - Reviews reviewsvc.Manager - CDC cdc.Source - Events cdcSubscriber + Projects projectsvc.Manager + Sessions controllers.SessionService + Activity controllers.ActivityRecorder + PRs prsvc.ActionManager + Reviews reviewsvc.Manager + Notifications controllers.NotificationService + CDC cdc.Source + Events cdcSubscriber } // API owns one controller per resource and is the single Register call the // router invokes to mount the /api/v1 surface. type API struct { - cfg config.Config - projects *controllers.ProjectsController - sessions *controllers.SessionsController - prs *controllers.PRsController - reviews *controllers.ReviewsController - events *EventsController + cfg config.Config + projects *controllers.ProjectsController + sessions *controllers.SessionsController + prs *controllers.PRsController + reviews *controllers.ReviewsController + notifications *controllers.NotificationsController + events *EventsController } // NewAPI constructs the API surface from its dependencies. cfg carries the @@ -51,9 +53,10 @@ func NewAPI(cfg config.Config, deps APIDeps) *API { Svc: deps.Sessions, Activity: deps.Activity, }, - prs: &controllers.PRsController{Svc: deps.PRs}, - reviews: &controllers.ReviewsController{Svc: deps.Reviews}, - events: &EventsController{Source: deps.CDC, Live: deps.Events}, + prs: &controllers.PRsController{Svc: deps.PRs}, + reviews: &controllers.ReviewsController{Svc: deps.Reviews}, + notifications: &controllers.NotificationsController{Svc: deps.Notifications}, + events: &EventsController{Source: deps.CDC, Live: deps.Events}, } } @@ -75,6 +78,7 @@ func (a *API) Register(root chi.Router) { a.sessions.Register(r) a.prs.Register(r) a.reviews.Register(r) + a.notifications.Register(r) // Sibling REST controllers plug in here. }) // Long-lived streams intentionally bypass the REST timeout middleware. diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index c91c02e3..45f95fde 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -51,6 +51,61 @@ paths: summary: Stream CDC events with durable replay tags: - events + /api/v1/notifications: + get: + operationId: listNotifications + parameters: + - description: Notification status filter. V1 supports only unread. + in: query + name: status + schema: + description: Notification status filter. V1 supports only unread. + enum: + - unread + type: string + - description: Optional project id filter. + in: query + name: projectId + schema: + description: Optional project id filter. + type: string + - description: Maximum notifications to return. Defaults to 50; capped at 100. + in: query + name: limit + schema: + description: Maximum notifications to return. Defaults to 50; capped at + 100. + maximum: 100 + minimum: 1 + type: integer + responses: + "200": + content: + application/json: + schema: + $ref: '#/components/schemas/ListNotificationsResponse' + description: OK + "400": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Bad Request + "500": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Internal Server Error + "501": + content: + application/json: + schema: + $ref: '#/components/schemas/APIError' + description: Not Implemented + summary: List unread notifications + tags: + - notifications /api/v1/orchestrators: get: operationId: listOrchestrators @@ -1186,6 +1241,15 @@ components: - ok - sessionId type: object + ListNotificationsResponse: + properties: + notifications: + items: + $ref: '#/components/schemas/NotificationResponse' + type: array + required: + - notifications + type: object ListProjectsResponse: properties: projects: @@ -1238,6 +1302,64 @@ components: - prNumber - method type: object + NotificationResponse: + properties: + body: + type: string + createdAt: + format: date-time + type: string + id: + type: string + prUrl: + type: string + projectId: + type: string + sessionId: + type: string + status: + enum: + - unread + - read + type: string + target: + $ref: '#/components/schemas/NotificationTarget' + title: + type: string + type: + enum: + - needs_input + - ready_to_merge + - pr_merged + - pr_closed_unmerged + type: string + required: + - id + - sessionId + - projectId + - prUrl + - type + - title + - body + - status + - createdAt + - target + type: object + NotificationTarget: + properties: + kind: + enum: + - session + - pr + type: string + prUrl: + type: string + sessionId: + type: string + required: + - kind + - sessionId + type: object OrchestratorResponse: properties: id: @@ -1691,5 +1813,7 @@ tags: name: prs - description: Code-review runs and findings name: reviews +- description: Durable dashboard notifications + name: notifications - description: Server-sent CDC event stream with durable replay name: events diff --git a/backend/internal/httpd/apispec/specgen/build.go b/backend/internal/httpd/apispec/specgen/build.go index 502cc252..803ef5cd 100644 --- a/backend/internal/httpd/apispec/specgen/build.go +++ b/backend/internal/httpd/apispec/specgen/build.go @@ -63,6 +63,8 @@ func Build() ([]byte, error) { "Pull-request actions (SCM lane)"), *(&openapi31.Tag{Name: "reviews"}).WithDescription( "Code-review runs and findings"), + *(&openapi31.Tag{Name: "notifications"}).WithDescription( + "Durable dashboard notifications"), *(&openapi31.Tag{Name: "events"}).WithDescription( "Server-sent CDC event stream with durable replay"), } @@ -155,6 +157,10 @@ var schemaNames = map[string]string{ "ControllersSpawnOrchestratorRequest": "SpawnOrchestratorRequest", "ControllersSpawnOrchestratorResponse": "SpawnOrchestratorResponse", "ControllersOrchestratorResponse": "OrchestratorResponse", + "ControllersListNotificationsQuery": "ListNotificationsQuery", + "ControllersNotificationTarget": "NotificationTarget", + "ControllersNotificationResponse": "NotificationResponse", + "ControllersListNotificationsResponse": "ListNotificationsResponse", // httpd/controllers — PR wire envelopes "ControllersMergePRResponse": "MergePRResponse", "ControllersResolveCommentsRequest": "ResolveCommentsRequest", @@ -252,9 +258,26 @@ func operations() []operation { ops = append(ops, sessionOperations()...) ops = append(ops, prOperations()...) ops = append(ops, reviewOperations()...) + ops = append(ops, notificationOperations()...) return ops } +func notificationOperations() []operation { + return []operation{ + { + method: http.MethodGet, path: "/api/v1/notifications", id: "listNotifications", tag: "notifications", + summary: "List unread notifications", + pathParams: []any{controllers.ListNotificationsQuery{}}, + resps: []respUnit{ + {http.StatusOK, controllers.ListNotificationsResponse{}}, + {http.StatusBadRequest, envelope.APIError{}}, + {http.StatusInternalServerError, envelope.APIError{}}, + {http.StatusNotImplemented, envelope.APIError{}}, + }, + }, + } +} + // reviewOperations declares the /reviews operations. Must stay 1:1 with the // routes ReviewsController.Register mounts (enforced by the parity test). func reviewOperations() []operation { diff --git a/backend/internal/httpd/controllers/dto.go b/backend/internal/httpd/controllers/dto.go index 557c42ee..2a03d65d 100644 --- a/backend/internal/httpd/controllers/dto.go +++ b/backend/internal/httpd/controllers/dto.go @@ -262,6 +262,39 @@ type OrchestratorResponse struct { ProjectName string `json:"projectName,omitempty"` } +// ListNotificationsQuery is the query string accepted by GET /api/v1/notifications. +type ListNotificationsQuery struct { + Status string `query:"status,omitempty" enum:"unread" description:"Notification status filter. V1 supports only unread."` + ProjectID string `query:"projectId,omitempty" description:"Optional project id filter."` + Limit int `query:"limit,omitempty" minimum:"1" maximum:"100" description:"Maximum notifications to return. Defaults to 50; capped at 100."` +} + +// NotificationTarget is the dashboard navigation target for a notification. +type NotificationTarget struct { + Kind string `json:"kind" enum:"session,pr"` + SessionID string `json:"sessionId"` + PRURL string `json:"prUrl,omitempty"` +} + +// NotificationResponse is one stored notification returned by the API. +type NotificationResponse struct { + ID string `json:"id"` + SessionID string `json:"sessionId"` + ProjectID string `json:"projectId"` + PRURL string `json:"prUrl"` + Type string `json:"type" enum:"needs_input,ready_to_merge,pr_merged,pr_closed_unmerged"` + Title string `json:"title"` + Body string `json:"body"` + Status string `json:"status" enum:"unread,read"` + CreatedAt time.Time `json:"createdAt"` + Target NotificationTarget `json:"target"` +} + +// ListNotificationsResponse is the body of GET /api/v1/notifications. +type ListNotificationsResponse struct { + Notifications []NotificationResponse `json:"notifications"` +} + // PRIDParam is the {id} path parameter shared by the /prs/{id} routes. type PRIDParam struct { ID string `path:"id" description:"PR number."` diff --git a/backend/internal/httpd/controllers/notifications.go b/backend/internal/httpd/controllers/notifications.go new file mode 100644 index 00000000..ac9ea934 --- /dev/null +++ b/backend/internal/httpd/controllers/notifications.go @@ -0,0 +1,102 @@ +package controllers + +import ( + "context" + "net/http" + "strconv" + + "github.com/go-chi/chi/v5" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope" + notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification" +) + +// NotificationService is the controller-facing notification service contract. +type NotificationService interface { + ListUnread(ctx context.Context, filter notificationsvc.ListFilter) ([]notificationsvc.Notification, error) +} + +// NotificationsController owns the /notifications routes. +type NotificationsController struct { + Svc NotificationService +} + +// Register mounts notification routes on the supplied router. +func (c *NotificationsController) Register(r chi.Router) { + r.Get("/notifications", c.list) +} + +func (c *NotificationsController) list(w http.ResponseWriter, r *http.Request) { + if c.Svc == nil { + apispec.NotImplemented(w, r, "GET", "/api/v1/notifications") + return + } + filter, err := parseNotificationListFilter(r) + if err != nil { + envelope.WriteAPIError(w, r, http.StatusBadRequest, "bad_request", "INVALID_QUERY", err.Error(), nil) + return + } + notifications, err := c.Svc.ListUnread(r.Context(), filter) + if err != nil { + envelope.WriteError(w, r, err) + return + } + envelope.WriteJSON(w, http.StatusOK, ListNotificationsResponse{Notifications: notificationResponses(notifications)}) +} + +func parseNotificationListFilter(r *http.Request) (notificationsvc.ListFilter, error) { + q := r.URL.Query() + status := q.Get("status") + if status == "" { + status = string(domain.NotificationUnread) + } + if status != string(domain.NotificationUnread) { + return notificationsvc.ListFilter{}, errNotificationStatusUnsupported + } + limit := notificationsvc.DefaultListLimit + if raw := q.Get("limit"); raw != "" { + parsed, err := strconv.Atoi(raw) + if err != nil || parsed <= 0 { + return notificationsvc.ListFilter{}, errNotificationLimitInvalid + } + limit = parsed + } + if limit > notificationsvc.MaxListLimit { + limit = notificationsvc.MaxListLimit + } + return notificationsvc.ListFilter{ProjectID: domain.ProjectID(q.Get("projectId")), Limit: limit}, nil +} + +var ( + errNotificationStatusUnsupported = notificationQueryError("status must be unread") + errNotificationLimitInvalid = notificationQueryError("limit must be a positive integer") +) + +type notificationQueryError string + +func (e notificationQueryError) Error() string { return string(e) } + +func notificationResponses(in []notificationsvc.Notification) []NotificationResponse { + out := make([]NotificationResponse, 0, len(in)) + for _, n := range in { + out = append(out, NotificationResponse{ + ID: n.ID, + SessionID: string(n.SessionID), + ProjectID: string(n.ProjectID), + PRURL: n.PRURL, + Type: string(n.Type), + Title: n.Title, + Body: n.Body, + Status: string(n.Status), + CreatedAt: n.CreatedAt, + Target: NotificationTarget{ + Kind: string(n.Target.Kind), + SessionID: string(n.Target.SessionID), + PRURL: n.Target.PRURL, + }, + }) + } + return out +} diff --git a/backend/internal/httpd/controllers/notifications_test.go b/backend/internal/httpd/controllers/notifications_test.go new file mode 100644 index 00000000..7323a12b --- /dev/null +++ b/backend/internal/httpd/controllers/notifications_test.go @@ -0,0 +1,97 @@ +package controllers_test + +import ( + "context" + "io" + "log/slog" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/config" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd" + "github.com/aoagents/agent-orchestrator/backend/internal/httpd/controllers" + notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification" +) + +type fakeNotificationService struct { + gotFilter notificationsvc.ListFilter + items []notificationsvc.Notification + err error +} + +func (f *fakeNotificationService) ListUnread(_ context.Context, filter notificationsvc.ListFilter) ([]notificationsvc.Notification, error) { + f.gotFilter = filter + return f.items, f.err +} + +func newNotificationTestServer(t *testing.T, svc controllers.NotificationService) *httptest.Server { + t.Helper() + log := slog.New(slog.NewTextHandler(io.Discard, nil)) + srv := httptest.NewServer(httpd.NewRouterWithControl(config.Config{}, log, nil, httpd.APIDeps{Notifications: svc}, httpd.ControlDeps{})) + t.Cleanup(srv.Close) + return srv +} + +func TestNotificationsAPI_ListUnread(t *testing.T) { + now := time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC) + svc := &fakeNotificationService{items: []notificationsvc.Notification{{ + NotificationRecord: domain.NotificationRecord{ID: "ntf_1", SessionID: "mer-1", ProjectID: "mer", Type: domain.NotificationNeedsInput, Title: "checkout-flow needs input", Body: "The agent is waiting for your response.", Status: domain.NotificationUnread, CreatedAt: now}, + Target: notificationsvc.Target{Kind: notificationsvc.TargetSession, SessionID: "mer-1"}, + }}} + srv := newNotificationTestServer(t, svc) + + body, status, _ := doRequest(t, srv, "GET", "/api/v1/notifications?projectId=mer&limit=10", "") + if status != http.StatusOK { + t.Fatalf("status = %d, want 200; body=%s", status, body) + } + if svc.gotFilter.ProjectID != "mer" || svc.gotFilter.Limit != 10 { + t.Fatalf("filter = %+v", svc.gotFilter) + } + var resp struct { + Notifications []struct { + ID string `json:"id"` + SessionID string `json:"sessionId"` + ProjectID string `json:"projectId"` + Type string `json:"type"` + Status string `json:"status"` + Target struct { + Kind string `json:"kind"` + SessionID string `json:"sessionId"` + } `json:"target"` + } `json:"notifications"` + } + mustJSON(t, body, &resp) + if len(resp.Notifications) != 1 || resp.Notifications[0].ID != "ntf_1" || resp.Notifications[0].Target.Kind != "session" { + t.Fatalf("resp = %+v", resp) + } +} + +func TestNotificationsAPI_DefaultsAndCapsLimit(t *testing.T) { + svc := &fakeNotificationService{} + srv := newNotificationTestServer(t, svc) + + _, status, _ := doRequest(t, srv, "GET", "/api/v1/notifications?limit=999", "") + if status != http.StatusOK { + t.Fatalf("status = %d, want 200", status) + } + if svc.gotFilter.Limit != notificationsvc.MaxListLimit { + t.Fatalf("limit = %d, want cap %d", svc.gotFilter.Limit, notificationsvc.MaxListLimit) + } +} + +func TestNotificationsAPI_RejectsUnsupportedStatus(t *testing.T) { + srv := newNotificationTestServer(t, &fakeNotificationService{}) + + body, status, _ := doRequest(t, srv, "GET", "/api/v1/notifications?status=read", "") + assertErrorCode(t, body, status, http.StatusBadRequest, "INVALID_QUERY") +} + +func TestNotificationsAPI_WithoutServiceIs501(t *testing.T) { + srv := newNotificationTestServer(t, nil) + + body, status, _ := doRequest(t, srv, "GET", "/api/v1/notifications", "") + assertErrorCode(t, body, status, http.StatusNotImplemented, "NOT_IMPLEMENTED") +} diff --git a/backend/internal/lifecycle/manager.go b/backend/internal/lifecycle/manager.go index 925af553..d7e2edd0 100644 --- a/backend/internal/lifecycle/manager.go +++ b/backend/internal/lifecycle/manager.go @@ -7,6 +7,7 @@ package lifecycle import ( "context" "fmt" + "log/slog" "sync" "time" @@ -23,11 +24,25 @@ type sessionStore interface { UpdatePRLastNudgeSignature(ctx context.Context, prURL, payload string) error } +// notificationSink is the optional lifecycle-to-notification-service boundary. +type notificationSink interface { + Notify(ctx context.Context, intent ports.NotificationIntent) error +} + +// Option customizes a Manager. +type Option func(*Manager) + +// WithNotificationSink wires lifecycle notification intents to a sink. +func WithNotificationSink(sink notificationSink) Option { + return func(m *Manager) { m.notifications = sink } +} + // Manager reduces runtime, activity, spawn, and termination observations into durable session facts. // It also owns agent nudges caused by PR observations, including merge-conflict, CI-failure, and review-feedback prompts. type Manager struct { - store sessionStore - messenger ports.AgentMessenger + store sessionStore + messenger ports.AgentMessenger + notifications notificationSink mu sync.Mutex window time.Duration @@ -36,8 +51,12 @@ type Manager struct { } // New builds a Lifecycle Manager over the session store it writes and the messenger it uses for agent nudges. -func New(store sessionStore, messenger ports.AgentMessenger) *Manager { - return &Manager{store: store, messenger: messenger, window: defaultRecentActivityWindow, clock: time.Now, react: newReactionState()} +func New(store sessionStore, messenger ports.AgentMessenger, opts ...Option) *Manager { + m := &Manager{store: store, messenger: messenger, window: defaultRecentActivityWindow, clock: time.Now, react: newReactionState()} + for _, opt := range opts { + opt(m) + } + return m } func (m *Manager) mutate(ctx context.Context, id domain.SessionID, fn func(domain.SessionRecord, time.Time) (domain.SessionRecord, bool)) error { @@ -79,29 +98,62 @@ func (m *Manager) ApplyActivitySignal(ctx context.Context, id domain.SessionID, if !s.Valid { return nil } - return m.mutate(ctx, id, func(cur domain.SessionRecord, now time.Time) (domain.SessionRecord, bool) { - if cur.IsTerminated { - return cur, false - } - next := cur - act := domain.Activity{State: s.State, LastActivityAt: timeOr(s.Timestamp, now)} - // A same-state repeat is still a write when it is the FIRST signal for - // this spawn: the receipt itself is a durable fact (it clears the - // no_signal display status). Hook deliveries are best-effort, so the - // first to ARRIVE may match the seeded state — e.g. a turn's "active" - // POST is lost and its Stop hook lands idle on the idle-seeded row. - if sameActivity(cur.Activity, act) && !cur.FirstSignalAt.IsZero() { - return cur, false - } - next.Activity = act - if next.FirstSignalAt.IsZero() { - next.FirstSignalAt = timeOr(s.Timestamp, now) - } - if s.State == domain.ActivityExited { - next.IsTerminated = true + var intent *ports.NotificationIntent + m.mu.Lock() + rec, ok, err := m.store.GetSession(ctx, id) + if err != nil || !ok { + m.mu.Unlock() + return err + } + now := m.clock() + if rec.IsTerminated { + m.mu.Unlock() + return nil + } + next := rec + act := domain.Activity{State: s.State, LastActivityAt: timeOr(s.Timestamp, now)} + // A same-state repeat is still a write when it is the FIRST signal for + // this spawn: the receipt itself is a durable fact (it clears the + // no_signal display status). Hook deliveries are best-effort, so the + // first to ARRIVE may match the seeded state — e.g. a turn's "active" + // POST is lost and its Stop hook lands idle on the idle-seeded row. + if sameActivity(rec.Activity, act) && !rec.FirstSignalAt.IsZero() { + m.mu.Unlock() + return nil + } + next.Activity = act + if next.FirstSignalAt.IsZero() { + next.FirstSignalAt = timeOr(s.Timestamp, now) + } + if s.State == domain.ActivityExited { + next.IsTerminated = true + } + next.UpdatedAt = now + if err := m.store.UpdateSession(ctx, next); err != nil { + m.mu.Unlock() + return err + } + if rec.Activity.State != domain.ActivityWaitingInput && next.Activity.State == domain.ActivityWaitingInput && !next.IsTerminated { + intent = &ports.NotificationIntent{ + Type: domain.NotificationNeedsInput, + SessionID: next.ID, + ProjectID: next.ProjectID, + CreatedAt: next.Activity.LastActivityAt, + SessionDisplayName: next.DisplayName, } - return next, true - }) + } + m.mu.Unlock() + m.emitNotification(ctx, intent) + return nil +} + +func (m *Manager) emitNotification(ctx context.Context, intent *ports.NotificationIntent) { + if intent == nil || m.notifications == nil { + return + } + if err := m.notifications.Notify(ctx, *intent); err != nil { + slog.Default().Warn("lifecycle: notification failed", "session", intent.SessionID, "type", intent.Type, "err", err) + } } // MarkSpawned marks a newly spawned or restored session live and stores runtime/workspace handles. diff --git a/backend/internal/lifecycle/manager_test.go b/backend/internal/lifecycle/manager_test.go index e739fa01..bf8dd2c3 100644 --- a/backend/internal/lifecycle/manager_test.go +++ b/backend/internal/lifecycle/manager_test.go @@ -546,3 +546,141 @@ func TestMarkSpawnedClearsFirstSignal(t *testing.T) { t.Fatalf("spawn/restore must clear the receipt, got %+v", got) } } + +type fakeNotificationSink struct { + intents []ports.NotificationIntent + err error +} + +func (f *fakeNotificationSink) Notify(_ context.Context, intent ports.NotificationIntent) error { + f.intents = append(f.intents, intent) + return f.err +} + +func TestActivity_WaitingInputTransitionEmitsNotification(t *testing.T) { + st := newFakeStore() + sink := &fakeNotificationSink{} + m := New(st, nil, WithNotificationSink(sink)) + now := time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC) + m.clock = func() time.Time { return now } + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", DisplayName: "checkout-flow", Activity: domain.Activity{State: domain.ActivityActive, LastActivityAt: now.Add(-time.Minute)}, FirstSignalAt: now.Add(-time.Minute)} + + if err := m.ApplyActivitySignal(ctx, "mer-1", ports.ActivitySignal{Valid: true, State: domain.ActivityWaitingInput}); err != nil { + t.Fatal(err) + } + if len(sink.intents) != 1 { + t.Fatalf("intents = %d, want 1", len(sink.intents)) + } + intent := sink.intents[0] + if intent.Type != domain.NotificationNeedsInput || intent.SessionID != "mer-1" || intent.ProjectID != "mer" || intent.SessionDisplayName != "checkout-flow" { + t.Fatalf("intent = %+v", intent) + } +} + +func TestActivity_WaitingInputSameStateDoesNotEmitNotification(t *testing.T) { + st := newFakeStore() + sink := &fakeNotificationSink{} + m := New(st, nil, WithNotificationSink(sink)) + now := time.Now() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Activity: domain.Activity{State: domain.ActivityWaitingInput, LastActivityAt: now}, FirstSignalAt: now} + + if err := m.ApplyActivitySignal(ctx, "mer-1", ports.ActivitySignal{Valid: true, State: domain.ActivityWaitingInput}); err != nil { + t.Fatal(err) + } + if len(sink.intents) != 0 { + t.Fatalf("same-state waiting_input emitted %+v", sink.intents) + } +} + +func TestActivity_TerminatedSessionDoesNotEmitNotification(t *testing.T) { + st := newFakeStore() + sink := &fakeNotificationSink{} + m := New(st, nil, WithNotificationSink(sink)) + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", IsTerminated: true, Activity: domain.Activity{State: domain.ActivityExited}} + + if err := m.ApplyActivitySignal(ctx, "mer-1", ports.ActivitySignal{Valid: true, State: domain.ActivityWaitingInput}); err != nil { + t.Fatal(err) + } + if len(sink.intents) != 0 { + t.Fatalf("terminated session emitted %+v", sink.intents) + } +} + +func TestSCMObservation_Notifications(t *testing.T) { + for _, tc := range []struct { + name string + obs ports.SCMObservation + want domain.NotificationType + }{ + { + name: "ready", + obs: ports.SCMObservation{Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1, Title: "checkout"}, CI: ports.SCMCIObservation{Summary: string(domain.CIPassing)}, Review: ports.SCMReviewObservation{Decision: string(domain.ReviewApproved)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, + want: domain.NotificationReadyToMerge, + }, + { + name: "merged", + obs: ports.SCMObservation{Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/2", Number: 2, Merged: true}}, + want: domain.NotificationPRMerged, + }, + { + name: "closed", + obs: ports.SCMObservation{Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/3", Number: 3, Closed: true}}, + want: domain.NotificationPRClosedUnmerged, + }, + } { + t.Run(tc.name, func(t *testing.T) { + st := newFakeStore() + sink := &fakeNotificationSink{} + m := New(st, nil, WithNotificationSink(sink)) + st.sessions["mer-1"] = working("mer-1") + if err := m.ApplySCMObservation(ctx, "mer-1", tc.obs); err != nil { + t.Fatal(err) + } + if len(sink.intents) != 1 { + t.Fatalf("intents = %d, want 1", len(sink.intents)) + } + if got := sink.intents[0]; got.Type != tc.want || got.PRURL != tc.obs.PR.URL || got.PRNumber != tc.obs.PR.Number { + t.Fatalf("intent = %+v, want type %s", got, tc.want) + } + }) + } +} + +func TestSCMObservation_NotReadyWhenCIOrReviewBlocks(t *testing.T) { + for _, obs := range []ports.SCMObservation{ + {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIFailing)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, + {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIPassing)}, Review: ports.SCMReviewObservation{Decision: string(domain.ReviewChangesRequest)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, + } { + st := newFakeStore() + sink := &fakeNotificationSink{} + m := New(st, nil, WithNotificationSink(sink)) + st.sessions["mer-1"] = working("mer-1") + if err := m.ApplySCMObservation(ctx, "mer-1", obs); err != nil { + t.Fatal(err) + } + if len(sink.intents) != 0 { + t.Fatalf("blocked PR emitted %+v", sink.intents) + } + } +} + +func TestSCMObservation_ReadyToMergeSuppressedWhileWaitingInput(t *testing.T) { + st := newFakeStore() + sink := &fakeNotificationSink{} + m := New(st, nil, WithNotificationSink(sink)) + rec := working("mer-1") + rec.Activity.State = domain.ActivityWaitingInput + st.sessions["mer-1"] = rec + obs := ports.SCMObservation{ + Fetched: true, + PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, + CI: ports.SCMCIObservation{Summary: string(domain.CIPassing)}, + Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}, + } + if err := m.ApplySCMObservation(ctx, "mer-1", obs); err != nil { + t.Fatal(err) + } + if len(sink.intents) != 0 { + t.Fatalf("waiting-input session emitted ready notification: %+v", sink.intents) + } +} diff --git a/backend/internal/lifecycle/reactions.go b/backend/internal/lifecycle/reactions.go index 614d3db9..c26d5b9f 100644 --- a/backend/internal/lifecycle/reactions.go +++ b/backend/internal/lifecycle/reactions.go @@ -92,7 +92,73 @@ func (m *Manager) ApplySCMObservation(ctx context.Context, id domain.SessionID, if !o.Fetched { return nil } - return m.ApplyPRObservation(ctx, id, scmToPRObservation(o)) + rec, ok, err := m.store.GetSession(ctx, id) + if err != nil || !ok { + return err + } + intent := m.notificationIntentForSCM(rec, o) + if err := m.ApplyPRObservation(ctx, id, scmToPRObservation(o)); err != nil { + return err + } + m.emitNotification(ctx, intent) + return nil +} + +func (m *Manager) notificationIntentForSCM(rec domain.SessionRecord, o ports.SCMObservation) *ports.NotificationIntent { + prURL := firstSCMNonEmpty(o.PR.URL, o.PR.HTMLURL) + base := ports.NotificationIntent{ + SessionID: rec.ID, + ProjectID: rec.ProjectID, + PRURL: prURL, + CreatedAt: timeOr(o.ObservedAt, m.clock()), + SessionDisplayName: rec.DisplayName, + PRNumber: o.PR.Number, + PRTitle: o.PR.Title, + PRSourceBranch: o.PR.SourceBranch, + PRTargetBranch: o.PR.TargetBranch, + Provider: o.Provider, + Repo: o.Repo, + } + if o.PR.Merged { + base.Type = domain.NotificationPRMerged + return &base + } + if o.PR.Closed && !o.PR.Merged { + base.Type = domain.NotificationPRClosedUnmerged + return &base + } + if rec.IsTerminated || rec.Activity.State == domain.ActivityWaitingInput || !scmObservationIsReadyToMerge(o) { + return nil + } + base.Type = domain.NotificationReadyToMerge + return &base +} + +func scmObservationIsReadyToMerge(o ports.SCMObservation) bool { + if o.PR.Merged || o.PR.Closed || o.PR.Draft { + return false + } + if domain.CIState(o.CI.Summary) == domain.CIFailing { + return false + } + if domain.ReviewDecision(o.Review.Decision) == domain.ReviewChangesRequest || hasUnresolvedSCMComments(o.Review.Threads) { + return false + } + return domain.Mergeability(o.Mergeability.State) == domain.MergeMergeable +} + +func hasUnresolvedSCMComments(threads []ports.SCMReviewThreadObservation) bool { + for _, th := range threads { + if th.Resolved || th.IsBot { + continue + } + for _, c := range th.Comments { + if !c.IsBot { + return true + } + } + } + return false } func scmToPRObservation(o ports.SCMObservation) ports.PRObservation { diff --git a/backend/internal/ports/notifications.go b/backend/internal/ports/notifications.go new file mode 100644 index 00000000..7f7c153e --- /dev/null +++ b/backend/internal/ports/notifications.go @@ -0,0 +1,27 @@ +package ports + +import ( + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// NotificationIntent is the lifecycle-to-notification-service contract. It is +// not an HTTP DTO; lifecycle fills it from facts it already has after the +// underlying session/PR state write succeeds. +type NotificationIntent struct { + Type domain.NotificationType + SessionID domain.SessionID + ProjectID domain.ProjectID + PRURL string + CreatedAt time.Time + + // Enrichment hints. These avoid storage reads on the hot path. + SessionDisplayName string + PRNumber int + PRTitle string + PRSourceBranch string + PRTargetBranch string + Provider string + Repo string +} diff --git a/backend/internal/service/notification/dispatcher.go b/backend/internal/service/notification/dispatcher.go new file mode 100644 index 00000000..32ca518e --- /dev/null +++ b/backend/internal/service/notification/dispatcher.go @@ -0,0 +1,32 @@ +package notification + +import "context" + +// Dispatcher is the deliberately tiny v1 delivery boundary. +type Dispatcher interface { + Dispatch(ctx context.Context, n Notification) error +} + +// DashboardPublisher publishes created notifications to dashboard transports. +type DashboardPublisher interface { + Publish(ctx context.Context, n Notification) error +} + +// DashboardDispatcher forwards created notifications to a dashboard publisher. +type DashboardDispatcher struct { + publisher DashboardPublisher +} + +// NewDashboardDispatcher builds a dispatcher around publisher. A nil publisher +// makes dispatch a no-op, which is the v1 daemon default until SSE is added. +func NewDashboardDispatcher(publisher DashboardPublisher) DashboardDispatcher { + return DashboardDispatcher{publisher: publisher} +} + +// Dispatch sends n to the dashboard publisher when one is configured. +func (d DashboardDispatcher) Dispatch(ctx context.Context, n Notification) error { + if d.publisher == nil { + return nil + } + return d.publisher.Publish(ctx, n) +} diff --git a/backend/internal/service/notification/enrich.go b/backend/internal/service/notification/enrich.go new file mode 100644 index 00000000..dc6729c0 --- /dev/null +++ b/backend/internal/service/notification/enrich.go @@ -0,0 +1,90 @@ +package notification + +import ( + "fmt" + "strings" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +func enrich(intent Intent) (domain.NotificationRecord, error) { + rec := domain.NotificationRecord{ + SessionID: intent.SessionID, + ProjectID: intent.ProjectID, + PRURL: strings.TrimSpace(intent.PRURL), + Type: intent.Type, + Status: domain.NotificationUnread, + CreatedAt: intent.CreatedAt, + } + if !intent.Type.Valid() { + return domain.NotificationRecord{}, domain.ErrInvalidNotificationType + } + if intent.Type != domain.NotificationNeedsInput && rec.PRURL == "" { + return domain.NotificationRecord{}, domain.ErrInvalidNotificationRecord + } + rec.Title = titleForIntent(intent) + rec.Body = bodyForIntent(intent) + if err := rec.Validate(); err != nil { + return domain.NotificationRecord{}, err + } + return rec, nil +} + +func titleForIntent(intent Intent) string { + switch intent.Type { + case domain.NotificationNeedsInput: + return fmt.Sprintf("%s needs input", sessionLabel(intent)) + case domain.NotificationReadyToMerge: + return fmt.Sprintf("%s is ready to merge", prLabel(intent)) + case domain.NotificationPRMerged: + return fmt.Sprintf("%s was merged", prLabel(intent)) + case domain.NotificationPRClosedUnmerged: + return fmt.Sprintf("%s was closed without merging", prLabel(intent)) + default: + return "Notification" + } +} + +func bodyForIntent(intent Intent) string { + switch intent.Type { + case domain.NotificationNeedsInput: + return "The agent is waiting for your response." + case domain.NotificationReadyToMerge: + if s := sessionLabel(intent); s != "session" { + return fmt.Sprintf("%s has no known blocking CI or review feedback.", s) + } + return "The pull request has no known blocking CI or review feedback." + case domain.NotificationPRMerged: + if title := strings.TrimSpace(intent.PRTitle); title != "" { + return fmt.Sprintf("%s was merged.", title) + } + return "The pull request was merged." + case domain.NotificationPRClosedUnmerged: + if title := strings.TrimSpace(intent.PRTitle); title != "" { + return fmt.Sprintf("%s was closed without merging.", title) + } + return "The pull request was closed without merging." + default: + return "" + } +} + +func sessionLabel(intent Intent) string { + if v := strings.TrimSpace(intent.SessionDisplayName); v != "" { + return v + } + if intent.SessionID != "" { + return string(intent.SessionID) + } + return "session" +} + +func prLabel(intent Intent) string { + if intent.PRNumber > 0 { + return fmt.Sprintf("PR #%d", intent.PRNumber) + } + if title := strings.TrimSpace(intent.PRTitle); title != "" { + return "PR " + title + } + return "PR" +} diff --git a/backend/internal/service/notification/service.go b/backend/internal/service/notification/service.go new file mode 100644 index 00000000..ac5099fe --- /dev/null +++ b/backend/internal/service/notification/service.go @@ -0,0 +1,119 @@ +package notification + +import ( + "context" + "errors" + "fmt" + "time" + + "github.com/google/uuid" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +const ( + // DefaultListLimit is the unread notification page size used when none is requested. + DefaultListLimit = 50 + // MaxListLimit caps unread notification API responses. + MaxListLimit = 100 +) + +// Manager validates lifecycle intents, enriches them into stored rows, persists +// unread notifications, and dispatches newly created rows. +type Manager struct { + store Store + dispatcher Dispatcher + clock func() time.Time + newID func() string +} + +// Deps configures a Manager. +type Deps struct { + Store Store + Dispatcher Dispatcher + Clock func() time.Time + NewID func() string +} + +// New constructs a Manager with production defaults for optional collaborators. +func New(d Deps) *Manager { + m := &Manager{store: d.Store, dispatcher: d.Dispatcher, clock: d.Clock, newID: d.NewID} + if m.clock == nil { + m.clock = time.Now + } + if m.newID == nil { + m.newID = func() string { return "ntf_" + uuid.NewString() } + } + return m +} + +// Notify stores and dispatches one notification intent. Duplicate unread rows +// are treated as a clean no-op. +func (m *Manager) Notify(ctx context.Context, intent Intent) error { + if m == nil || m.store == nil { + return errors.New("notification: store is required") + } + if intent.CreatedAt.IsZero() { + intent.CreatedAt = m.clock().UTC() + } + rec, err := enrich(intent) + if err != nil { + return fmt.Errorf("notification enrich: %w", err) + } + rec.ID = m.newID() + created, inserted, err := m.store.CreateNotification(ctx, rec) + if err != nil { + return fmt.Errorf("notification store: %w", err) + } + if !inserted { + return nil + } + if m.dispatcher == nil { + return nil + } + return m.dispatcher.Dispatch(ctx, notificationFromRecord(created)) +} + +// ListUnread returns unread notifications newest-first, optionally limited to a project. +func (m *Manager) ListUnread(ctx context.Context, filter ListFilter) ([]Notification, error) { + if m == nil || m.store == nil { + return nil, errors.New("notification: store is required") + } + limit := normalizeLimit(filter.Limit) + var rows []domain.NotificationRecord + var err error + if filter.ProjectID != "" { + rows, err = m.store.ListUnreadNotificationsByProject(ctx, filter.ProjectID, limit) + } else { + rows, err = m.store.ListUnreadNotifications(ctx, limit) + } + if err != nil { + return nil, err + } + out := make([]Notification, 0, len(rows)) + for _, row := range rows { + out = append(out, notificationFromRecord(row)) + } + return out, nil +} + +func normalizeLimit(limit int) int { + if limit <= 0 { + return DefaultListLimit + } + if limit > MaxListLimit { + return MaxListLimit + } + return limit +} + +func notificationFromRecord(rec domain.NotificationRecord) Notification { + return Notification{NotificationRecord: rec, Target: targetForRecord(rec)} +} + +func targetForRecord(rec domain.NotificationRecord) Target { + if rec.PRURL != "" { + return Target{Kind: TargetPR, SessionID: rec.SessionID, PRURL: rec.PRURL} + } + return Target{Kind: TargetSession, SessionID: rec.SessionID} +} diff --git a/backend/internal/service/notification/service_test.go b/backend/internal/service/notification/service_test.go new file mode 100644 index 00000000..a676b7c2 --- /dev/null +++ b/backend/internal/service/notification/service_test.go @@ -0,0 +1,127 @@ +package notification + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +type fakeStore struct { + rows []domain.NotificationRecord + duplicate bool + err error +} + +func (f *fakeStore) CreateNotification(_ context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) { + if f.err != nil { + return domain.NotificationRecord{}, false, f.err + } + if f.duplicate { + return domain.NotificationRecord{}, false, nil + } + f.rows = append(f.rows, rec) + return rec, true, nil +} + +func (f *fakeStore) ListUnreadNotifications(_ context.Context, _ int) ([]domain.NotificationRecord, error) { + return f.rows, nil +} + +func (f *fakeStore) ListUnreadNotificationsByProject(_ context.Context, projectID domain.ProjectID, _ int) ([]domain.NotificationRecord, error) { + out := make([]domain.NotificationRecord, 0, len(f.rows)) + for _, row := range f.rows { + if row.ProjectID == projectID { + out = append(out, row) + } + } + return out, nil +} + +type captureDispatcher struct { + notifications []Notification + err error +} + +func (d *captureDispatcher) Dispatch(_ context.Context, n Notification) error { + d.notifications = append(d.notifications, n) + return d.err +} + +func TestManagerNotifyPersistsThenDispatches(t *testing.T) { + st := &fakeStore{} + dispatch := &captureDispatcher{} + now := time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC) + mgr := New(Deps{Store: st, Dispatcher: dispatch, Clock: func() time.Time { return now }, NewID: func() string { return "ntf_1" }}) + + if err := mgr.Notify(context.Background(), Intent{ + Type: domain.NotificationNeedsInput, + SessionID: "mer-1", + ProjectID: "mer", + SessionDisplayName: "checkout-flow", + }); err != nil { + t.Fatalf("Notify: %v", err) + } + if len(st.rows) != 1 { + t.Fatalf("stored rows = %d, want 1", len(st.rows)) + } + if got := st.rows[0]; got.ID != "ntf_1" || got.CreatedAt != now || got.Status != domain.NotificationUnread || got.Title != "checkout-flow needs input" { + t.Fatalf("stored notification = %+v", got) + } + if len(dispatch.notifications) != 1 || dispatch.notifications[0].ID != "ntf_1" { + t.Fatalf("dispatch = %+v", dispatch.notifications) + } +} + +func TestManagerNotifyDuplicateDoesNotDispatch(t *testing.T) { + st := &fakeStore{duplicate: true} + dispatch := &captureDispatcher{} + mgr := New(Deps{Store: st, Dispatcher: dispatch, Clock: func() time.Time { return time.Now() }, NewID: func() string { return "ntf_1" }}) + + err := mgr.Notify(context.Background(), Intent{Type: domain.NotificationNeedsInput, SessionID: "mer-1", ProjectID: "mer", CreatedAt: time.Now()}) + if err != nil { + t.Fatalf("Notify duplicate: %v", err) + } + if len(dispatch.notifications) != 0 { + t.Fatalf("duplicate should not dispatch, got %+v", dispatch.notifications) + } +} + +func TestManagerNotifyDispatchFailureLeavesStoredRow(t *testing.T) { + st := &fakeStore{} + dispatch := &captureDispatcher{err: errors.New("sse down")} + mgr := New(Deps{Store: st, Dispatcher: dispatch, Clock: func() time.Time { return time.Now() }, NewID: func() string { return "ntf_1" }}) + + err := mgr.Notify(context.Background(), Intent{Type: domain.NotificationNeedsInput, SessionID: "mer-1", ProjectID: "mer", CreatedAt: time.Now()}) + if err == nil { + t.Fatal("want dispatch error") + } + if len(st.rows) != 1 { + t.Fatalf("row should remain stored after dispatch failure; rows=%d", len(st.rows)) + } +} + +func TestManagerNotifyRejectsUnknownType(t *testing.T) { + mgr := New(Deps{Store: &fakeStore{}, Clock: func() time.Time { return time.Now() }}) + err := mgr.Notify(context.Background(), Intent{Type: "surprise", SessionID: "mer-1", ProjectID: "mer"}) + if !errors.Is(err, domain.ErrInvalidNotificationType) { + t.Fatalf("err = %v, want invalid type", err) + } +} + +func TestListUnreadAddsTargets(t *testing.T) { + st := &fakeStore{rows: []domain.NotificationRecord{ + {ID: "n1", SessionID: "mer-1", ProjectID: "mer", Type: domain.NotificationNeedsInput, Title: "needs", Status: domain.NotificationUnread, CreatedAt: time.Now()}, + {ID: "n2", SessionID: "mer-1", ProjectID: "mer", PRURL: "https://github.com/o/r/pull/1", Type: domain.NotificationReadyToMerge, Title: "ready", Status: domain.NotificationUnread, CreatedAt: time.Now()}, + }} + mgr := New(Deps{Store: st}) + got, err := mgr.ListUnread(context.Background(), ListFilter{Limit: 10}) + if err != nil { + t.Fatalf("ListUnread: %v", err) + } + if got[0].Target.Kind != TargetSession || got[1].Target.Kind != TargetPR || got[1].Target.PRURL == "" { + t.Fatalf("targets = %+v", got) + } +} diff --git a/backend/internal/service/notification/store.go b/backend/internal/service/notification/store.go new file mode 100644 index 00000000..296e6aec --- /dev/null +++ b/backend/internal/service/notification/store.go @@ -0,0 +1,14 @@ +package notification + +import ( + "context" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// Store is the notification service's persistence surface. +type Store interface { + CreateNotification(ctx context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) + ListUnreadNotifications(ctx context.Context, limit int) ([]domain.NotificationRecord, error) + ListUnreadNotificationsByProject(ctx context.Context, projectID domain.ProjectID, limit int) ([]domain.NotificationRecord, error) +} diff --git a/backend/internal/service/notification/types.go b/backend/internal/service/notification/types.go new file mode 100644 index 00000000..705c6d4d --- /dev/null +++ b/backend/internal/service/notification/types.go @@ -0,0 +1,41 @@ +// Package notification enriches lifecycle notification intents, persists unread +// rows, and dispatches created notifications to dashboard-facing publishers. +package notification + +import ( + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + "github.com/aoagents/agent-orchestrator/backend/internal/ports" +) + +// Intent is the service boundary shape. It aliases the lifecycle contract so +// Manager.Notify also satisfies lifecycle's NotificationSink without copying. +type Intent = ports.NotificationIntent + +// TargetKind describes what a dashboard should navigate to for a notification. +type TargetKind string + +const ( + // TargetSession navigates to a session detail view. + TargetSession TargetKind = "session" + // TargetPR navigates to a pull request view. + TargetPR TargetKind = "pr" +) + +// Target is the service-facing navigation metadata for a notification. +type Target struct { + Kind TargetKind + SessionID domain.SessionID + PRURL string +} + +// Notification is the dashboard-facing service DTO assembled from a stored row. +type Notification struct { + domain.NotificationRecord + Target Target +} + +// ListFilter controls unread notification listing. +type ListFilter struct { + ProjectID domain.ProjectID + Limit int +} diff --git a/backend/internal/storage/sqlite/gen/models.go b/backend/internal/storage/sqlite/gen/models.go index f8d614bf..26610fe6 100644 --- a/backend/internal/storage/sqlite/gen/models.go +++ b/backend/internal/storage/sqlite/gen/models.go @@ -21,6 +21,18 @@ type ChangeLog struct { CreatedAt time.Time } +type Notification struct { + ID string + SessionID domain.SessionID + ProjectID domain.ProjectID + PRURL string + Type domain.NotificationType + Title string + Body string + Status domain.NotificationStatus + CreatedAt time.Time +} + type PR struct { URL string SessionID domain.SessionID diff --git a/backend/internal/storage/sqlite/gen/notifications.sql.go b/backend/internal/storage/sqlite/gen/notifications.sql.go new file mode 100644 index 00000000..86036e6f --- /dev/null +++ b/backend/internal/storage/sqlite/gen/notifications.sql.go @@ -0,0 +1,146 @@ +// Code generated by sqlc. DO NOT EDIT. +// versions: +// sqlc v1.31.1 +// source: notifications.sql + +package gen + +import ( + "context" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +const createNotification = `-- name: CreateNotification :one +INSERT INTO notifications ( + id, session_id, project_id, pr_url, type, title, body, status, created_at +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) +RETURNING id, session_id, project_id, pr_url, type, title, body, status, created_at +` + +type CreateNotificationParams struct { + ID string + SessionID domain.SessionID + ProjectID domain.ProjectID + PRURL string + Type domain.NotificationType + Title string + Body string + Status domain.NotificationStatus + CreatedAt time.Time +} + +func (q *Queries) CreateNotification(ctx context.Context, arg CreateNotificationParams) (Notification, error) { + row := q.db.QueryRowContext(ctx, createNotification, + arg.ID, + arg.SessionID, + arg.ProjectID, + arg.PRURL, + arg.Type, + arg.Title, + arg.Body, + arg.Status, + arg.CreatedAt, + ) + var i Notification + err := row.Scan( + &i.ID, + &i.SessionID, + &i.ProjectID, + &i.PRURL, + &i.Type, + &i.Title, + &i.Body, + &i.Status, + &i.CreatedAt, + ) + return i, err +} + +const listUnreadNotifications = `-- name: ListUnreadNotifications :many +SELECT id, session_id, project_id, pr_url, type, title, body, status, created_at +FROM notifications +WHERE status = 'unread' +ORDER BY created_at DESC +LIMIT ? +` + +func (q *Queries) ListUnreadNotifications(ctx context.Context, limit int64) ([]Notification, error) { + rows, err := q.db.QueryContext(ctx, listUnreadNotifications, limit) + if err != nil { + return nil, err + } + defer rows.Close() + items := []Notification{} + for rows.Next() { + var i Notification + if err := rows.Scan( + &i.ID, + &i.SessionID, + &i.ProjectID, + &i.PRURL, + &i.Type, + &i.Title, + &i.Body, + &i.Status, + &i.CreatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const listUnreadNotificationsByProject = `-- name: ListUnreadNotificationsByProject :many +SELECT id, session_id, project_id, pr_url, type, title, body, status, created_at +FROM notifications +WHERE project_id = ? AND status = 'unread' +ORDER BY created_at DESC +LIMIT ? +` + +type ListUnreadNotificationsByProjectParams struct { + ProjectID domain.ProjectID + Limit int64 +} + +func (q *Queries) ListUnreadNotificationsByProject(ctx context.Context, arg ListUnreadNotificationsByProjectParams) ([]Notification, error) { + rows, err := q.db.QueryContext(ctx, listUnreadNotificationsByProject, arg.ProjectID, arg.Limit) + if err != nil { + return nil, err + } + defer rows.Close() + items := []Notification{} + for rows.Next() { + var i Notification + if err := rows.Scan( + &i.ID, + &i.SessionID, + &i.ProjectID, + &i.PRURL, + &i.Type, + &i.Title, + &i.Body, + &i.Status, + &i.CreatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} diff --git a/backend/internal/storage/sqlite/migrations/0011_notifications.sql b/backend/internal/storage/sqlite/migrations/0011_notifications.sql new file mode 100644 index 00000000..0612952b --- /dev/null +++ b/backend/internal/storage/sqlite/migrations/0011_notifications.sql @@ -0,0 +1,43 @@ +-- +goose Up +-- +goose StatementBegin +CREATE TABLE notifications ( + id TEXT PRIMARY KEY, + session_id TEXT NOT NULL REFERENCES sessions(id) ON DELETE CASCADE, + project_id TEXT NOT NULL REFERENCES projects(id) ON DELETE CASCADE, + pr_url TEXT NOT NULL DEFAULT '', + type TEXT NOT NULL CHECK ( + type IN ( + 'needs_input', + 'ready_to_merge', + 'pr_merged', + 'pr_closed_unmerged' + ) + ), + title TEXT NOT NULL, + body TEXT NOT NULL DEFAULT '', + status TEXT NOT NULL DEFAULT 'unread' CHECK (status IN ('read', 'unread')), + created_at TIMESTAMP NOT NULL +); + +CREATE INDEX idx_notifications_unread + ON notifications(status, created_at DESC); + +CREATE INDEX idx_notifications_project_unread + ON notifications(project_id, status, created_at DESC); + +CREATE INDEX idx_notifications_session + ON notifications(session_id, created_at DESC); + +CREATE UNIQUE INDEX idx_notifications_unread_dedupe + ON notifications(session_id, type, pr_url) + WHERE status = 'unread'; +-- +goose StatementEnd + +-- +goose Down +-- +goose StatementBegin +DROP INDEX IF EXISTS idx_notifications_unread_dedupe; +DROP INDEX IF EXISTS idx_notifications_session; +DROP INDEX IF EXISTS idx_notifications_project_unread; +DROP INDEX IF EXISTS idx_notifications_unread; +DROP TABLE IF EXISTS notifications; +-- +goose StatementEnd diff --git a/backend/internal/storage/sqlite/queries/notifications.sql b/backend/internal/storage/sqlite/queries/notifications.sql new file mode 100644 index 00000000..8a8e0e44 --- /dev/null +++ b/backend/internal/storage/sqlite/queries/notifications.sql @@ -0,0 +1,19 @@ +-- name: CreateNotification :one +INSERT INTO notifications ( + id, session_id, project_id, pr_url, type, title, body, status, created_at +) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?) +RETURNING *; + +-- name: ListUnreadNotifications :many +SELECT * +FROM notifications +WHERE status = 'unread' +ORDER BY created_at DESC +LIMIT ?; + +-- name: ListUnreadNotificationsByProject :many +SELECT * +FROM notifications +WHERE project_id = ? AND status = 'unread' +ORDER BY created_at DESC +LIMIT ?; diff --git a/backend/internal/storage/sqlite/store/notification_store.go b/backend/internal/storage/sqlite/store/notification_store.go new file mode 100644 index 00000000..68b7682e --- /dev/null +++ b/backend/internal/storage/sqlite/store/notification_store.go @@ -0,0 +1,96 @@ +package store + +import ( + "context" + "errors" + "fmt" + "strings" + + moderncsqlite "modernc.org/sqlite" + sqlite3 "modernc.org/sqlite/lib" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" + notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification" + "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite/gen" +) + +var _ notificationsvc.Store = (*Store)(nil) + +// CreateNotification inserts one unread notification. It returns created=false +// when the unread dedupe index already has a matching row. +func (s *Store) CreateNotification(ctx context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) { + if err := rec.Validate(); err != nil { + return domain.NotificationRecord{}, false, err + } + s.writeMu.Lock() + defer s.writeMu.Unlock() + row, err := s.qw.CreateNotification(ctx, gen.CreateNotificationParams{ + ID: rec.ID, + SessionID: rec.SessionID, + ProjectID: rec.ProjectID, + PRURL: rec.PRURL, + Type: rec.Type, + Title: rec.Title, + Body: rec.Body, + Status: rec.Status, + CreatedAt: rec.CreatedAt, + }) + if err != nil { + if isUnreadNotificationDuplicate(err) { + return domain.NotificationRecord{}, false, nil + } + return domain.NotificationRecord{}, false, fmt.Errorf("create notification %s: %w", rec.ID, err) + } + return notificationFromGen(row), true, nil +} + +// ListUnreadNotifications returns unread notifications newest-first. +func (s *Store) ListUnreadNotifications(ctx context.Context, limit int) ([]domain.NotificationRecord, error) { + rows, err := s.qr.ListUnreadNotifications(ctx, int64(limit)) + if err != nil { + return nil, fmt.Errorf("list unread notifications: %w", err) + } + return notificationsFromGen(rows), nil +} + +// ListUnreadNotificationsByProject returns unread notifications for one project newest-first. +func (s *Store) ListUnreadNotificationsByProject(ctx context.Context, projectID domain.ProjectID, limit int) ([]domain.NotificationRecord, error) { + rows, err := s.qr.ListUnreadNotificationsByProject(ctx, gen.ListUnreadNotificationsByProjectParams{ProjectID: projectID, Limit: int64(limit)}) + if err != nil { + return nil, fmt.Errorf("list unread notifications for %s: %w", projectID, err) + } + return notificationsFromGen(rows), nil +} + +func isUnreadNotificationDuplicate(err error) bool { + var sqliteErr *moderncsqlite.Error + if !errors.As(err, &sqliteErr) || sqliteErr.Code() != sqlite3.SQLITE_CONSTRAINT_UNIQUE { + return false + } + msg := err.Error() + return strings.Contains(msg, "notifications.session_id") && + strings.Contains(msg, "notifications.type") && + strings.Contains(msg, "notifications.pr_url") +} + +func notificationFromGen(row gen.Notification) domain.NotificationRecord { + return domain.NotificationRecord{ + ID: row.ID, + SessionID: row.SessionID, + ProjectID: row.ProjectID, + PRURL: row.PRURL, + Type: row.Type, + Title: row.Title, + Body: row.Body, + Status: row.Status, + CreatedAt: row.CreatedAt, + } +} + +func notificationsFromGen(rows []gen.Notification) []domain.NotificationRecord { + out := make([]domain.NotificationRecord, 0, len(rows)) + for _, row := range rows { + out = append(out, notificationFromGen(row)) + } + return out +} diff --git a/backend/internal/storage/sqlite/store/notification_store_test.go b/backend/internal/storage/sqlite/store/notification_store_test.go new file mode 100644 index 00000000..77c02674 --- /dev/null +++ b/backend/internal/storage/sqlite/store/notification_store_test.go @@ -0,0 +1,90 @@ +package store_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +func TestNotificationStore_InsertListAndDedupe(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + seedProject(t, s, "mer") + sess, err := s.CreateSession(ctx, sampleRecord("mer")) + if err != nil { + t.Fatalf("create session: %v", err) + } + now := time.Now().UTC().Truncate(time.Second) + rec := domain.NotificationRecord{ + ID: "ntf_1", + SessionID: sess.ID, + ProjectID: sess.ProjectID, + Type: domain.NotificationNeedsInput, + Title: "checkout-flow needs input", + Status: domain.NotificationUnread, + CreatedAt: now, + } + created, inserted, err := s.CreateNotification(ctx, rec) + if err != nil || !inserted { + t.Fatalf("CreateNotification inserted=%v err=%v", inserted, err) + } + if created.ID != rec.ID || created.Title != rec.Title { + t.Fatalf("created = %+v", created) + } + dup := rec + dup.ID = "ntf_2" + _, inserted, err = s.CreateNotification(ctx, dup) + if err != nil || inserted { + t.Fatalf("duplicate inserted=%v err=%v, want false nil", inserted, err) + } + rows, err := s.ListUnreadNotifications(ctx, 10) + if err != nil { + t.Fatalf("ListUnreadNotifications: %v", err) + } + if len(rows) != 1 || rows[0].ID != "ntf_1" { + t.Fatalf("rows = %+v", rows) + } +} + +func TestNotificationStore_ProjectFilterAndNewestFirst(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + seedProject(t, s, "mer") + seedProject(t, s, "ao") + mer, _ := s.CreateSession(ctx, sampleRecord("mer")) + ao, _ := s.CreateSession(ctx, sampleRecord("ao")) + base := time.Now().UTC().Truncate(time.Second) + for _, rec := range []domain.NotificationRecord{ + {ID: "old", SessionID: mer.ID, ProjectID: mer.ProjectID, Type: domain.NotificationNeedsInput, Title: "old", Status: domain.NotificationUnread, CreatedAt: base}, + {ID: "new", SessionID: mer.ID, ProjectID: mer.ProjectID, PRURL: "https://github.com/o/r/pull/1", Type: domain.NotificationReadyToMerge, Title: "new", Status: domain.NotificationUnread, CreatedAt: base.Add(time.Minute)}, + {ID: "other", SessionID: ao.ID, ProjectID: ao.ProjectID, Type: domain.NotificationNeedsInput, Title: "other", Status: domain.NotificationUnread, CreatedAt: base.Add(2 * time.Minute)}, + } { + if _, inserted, err := s.CreateNotification(ctx, rec); err != nil || !inserted { + t.Fatalf("insert %s inserted=%v err=%v", rec.ID, inserted, err) + } + } + rows, err := s.ListUnreadNotificationsByProject(ctx, "mer", 10) + if err != nil { + t.Fatalf("ListUnreadNotificationsByProject: %v", err) + } + if len(rows) != 2 || rows[0].ID != "new" || rows[1].ID != "old" { + t.Fatalf("project rows = %+v", rows) + } +} + +func TestNotificationStore_CheckConstraintRejectsInvalidStatus(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + seedProject(t, s, "mer") + sess, _ := s.CreateSession(ctx, sampleRecord("mer")) + _, _, err := s.CreateNotification(ctx, domain.NotificationRecord{ + ID: "bad", SessionID: sess.ID, ProjectID: sess.ProjectID, Type: domain.NotificationNeedsInput, + Title: "bad", Status: "archived", CreatedAt: time.Now(), + }) + if !errors.Is(err, domain.ErrInvalidNotificationStatus) { + t.Fatalf("err = %v, want invalid status", err) + } +} diff --git a/backend/sqlc.yaml b/backend/sqlc.yaml index 1813a599..4baabf6b 100644 --- a/backend/sqlc.yaml +++ b/backend/sqlc.yaml @@ -56,6 +56,22 @@ sql: type: "PRCheckStatus" - column: "pr_comment.resolved" go_type: "bool" + - column: "notifications.session_id" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "SessionID" + - column: "notifications.project_id" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "ProjectID" + - column: "notifications.type" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "NotificationType" + - column: "notifications.status" + go_type: + import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" + type: "NotificationStatus" - column: "projects.id" go_type: import: "github.com/aoagents/agent-orchestrator/backend/internal/domain" diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 93d306ad..3fe5cd03 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -21,6 +21,23 @@ export interface paths { patch?: never; trace?: never; }; + "/api/v1/notifications": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** List unread notifications */ + get: operations["listNotifications"]; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; "/api/v1/orchestrators": { parameters: { query?: never; @@ -431,6 +448,9 @@ export interface components { ok: boolean; sessionId: string; }; + ListNotificationsResponse: { + notifications: components["schemas"]["NotificationResponse"][]; + }; ListProjectsResponse: { projects: components["schemas"]["ProjectSummary"][]; }; @@ -449,6 +469,27 @@ export interface components { ok: boolean; prNumber: number; }; + NotificationResponse: { + body: string; + /** Format: date-time */ + createdAt: string; + id: string; + prUrl: string; + projectId: string; + sessionId: string; + /** @enum {string} */ + status: "unread" | "read"; + target: components["schemas"]["NotificationTarget"]; + title: string; + /** @enum {string} */ + type: "needs_input" | "ready_to_merge" | "pr_merged" | "pr_closed_unmerged"; + }; + NotificationTarget: { + /** @enum {string} */ + kind: "session" | "pr"; + prUrl?: string; + sessionId: string; + }; OrchestratorResponse: { id: string; projectId: string; @@ -678,6 +719,60 @@ export interface operations { }; }; }; + listNotifications: { + parameters: { + query?: { + /** @description Notification status filter. V1 supports only unread. */ + status?: "unread"; + /** @description Optional project id filter. */ + projectId?: string; + /** @description Maximum notifications to return. Defaults to 50; capped at 100. */ + limit?: number; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ListNotificationsResponse"]; + }; + }; + /** @description Bad Request */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Internal Server Error */ + 500: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + /** @description Not Implemented */ + 501: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["APIError"]; + }; + }; + }; + }; listOrchestrators: { parameters: { query?: never; From 916a45dd43d6c5289d6e58a37cfb2d4cb544a135 Mon Sep 17 00:00:00 2001 From: whoisasx Date: Thu, 11 Jun 2026 17:50:07 +0530 Subject: [PATCH 2/6] fix: address notification review feedback --- .../internal/httpd/controllers/sessions.go | 4 ++ .../controllers/sessions_activity_test.go | 7 ++++ backend/internal/lifecycle/manager.go | 6 ++- backend/internal/lifecycle/manager_test.go | 8 ++++ backend/internal/lifecycle/reactions.go | 25 +++++++++--- backend/internal/ports/session.go | 9 ++++- .../storage/sqlite/gen/notifications.sql.go | 30 ++++++++++++++ .../storage/sqlite/queries/notifications.sql | 6 +++ .../sqlite/store/notification_store.go | 40 ++++++++++++++----- 9 files changed, 117 insertions(+), 18 deletions(-) diff --git a/backend/internal/httpd/controllers/sessions.go b/backend/internal/httpd/controllers/sessions.go index bf033584..3d0b2bb9 100644 --- a/backend/internal/httpd/controllers/sessions.go +++ b/backend/internal/httpd/controllers/sessions.go @@ -304,6 +304,10 @@ func (c *SessionsController) activity(w http.ResponseWriter, r *http.Request) { return } if err := c.Activity.ApplyActivitySignal(r.Context(), sessionID(r), ports.ActivitySignal{Valid: true, State: state}); err != nil { + if errors.Is(err, ports.ErrSessionNotFound) { + envelope.WriteAPIError(w, r, http.StatusNotFound, "not_found", "SESSION_NOT_FOUND", "Unknown session", nil) + return + } envelope.WriteError(w, r, err) return } diff --git a/backend/internal/httpd/controllers/sessions_activity_test.go b/backend/internal/httpd/controllers/sessions_activity_test.go index ed63dc4b..5d7b6694 100644 --- a/backend/internal/httpd/controllers/sessions_activity_test.go +++ b/backend/internal/httpd/controllers/sessions_activity_test.go @@ -84,6 +84,13 @@ func TestSessionsAPI_ActivityRejectsBadJSON(t *testing.T) { assertErrorCode(t, body, status, http.StatusBadRequest, "INVALID_JSON") } +func TestSessionsAPI_ActivityMissingSessionIs404(t *testing.T) { + srv := newActivityTestServer(t, &fakeActivityRecorder{err: ports.ErrSessionNotFound}) + + body, status, _ := doRequest(t, srv, "POST", "/api/v1/sessions/missing/activity", `{"state":"idle"}`) + assertErrorCode(t, body, status, http.StatusNotFound, "SESSION_NOT_FOUND") +} + func TestSessionsAPI_ActivityRecorderErrorIs500(t *testing.T) { srv := newActivityTestServer(t, &fakeActivityRecorder{err: errors.New("boom")}) diff --git a/backend/internal/lifecycle/manager.go b/backend/internal/lifecycle/manager.go index d7e2edd0..738051a9 100644 --- a/backend/internal/lifecycle/manager.go +++ b/backend/internal/lifecycle/manager.go @@ -101,10 +101,14 @@ func (m *Manager) ApplyActivitySignal(ctx context.Context, id domain.SessionID, var intent *ports.NotificationIntent m.mu.Lock() rec, ok, err := m.store.GetSession(ctx, id) - if err != nil || !ok { + if err != nil { m.mu.Unlock() return err } + if !ok { + m.mu.Unlock() + return fmt.Errorf("%w: %s", ports.ErrSessionNotFound, id) + } now := m.clock() if rec.IsTerminated { m.mu.Unlock() diff --git a/backend/internal/lifecycle/manager_test.go b/backend/internal/lifecycle/manager_test.go index bf8dd2c3..b43f2539 100644 --- a/backend/internal/lifecycle/manager_test.go +++ b/backend/internal/lifecycle/manager_test.go @@ -112,6 +112,14 @@ func TestActivity_InvalidIsIgnored(t *testing.T) { } } +func TestActivity_MissingSessionReturnsNotFound(t *testing.T) { + m, _, _ := newManager() + err := m.ApplyActivitySignal(ctx, "missing-1", ports.ActivitySignal{Valid: true, State: domain.ActivityWaitingInput}) + if !errors.Is(err, ports.ErrSessionNotFound) { + t.Fatalf("err = %v, want ErrSessionNotFound", err) + } +} + func TestMarkTerminated(t *testing.T) { m, st, _ := newManager() st.sessions["mer-1"] = working("mer-1") diff --git a/backend/internal/lifecycle/reactions.go b/backend/internal/lifecycle/reactions.go index c26d5b9f..ad8ef8ac 100644 --- a/backend/internal/lifecycle/reactions.go +++ b/backend/internal/lifecycle/reactions.go @@ -3,6 +3,7 @@ package lifecycle import ( "context" "encoding/json" + "fmt" "log/slog" "strings" "sync" @@ -92,18 +93,32 @@ func (m *Manager) ApplySCMObservation(ctx context.Context, id domain.SessionID, if !o.Fetched { return nil } - rec, ok, err := m.store.GetSession(ctx, id) - if err != nil || !ok { + if err := m.ApplyPRObservation(ctx, id, scmToPRObservation(o)); err != nil { return err } - intent := m.notificationIntentForSCM(rec, o) - if err := m.ApplyPRObservation(ctx, id, scmToPRObservation(o)); err != nil { + intent, err := m.notificationIntentForCurrentSCM(ctx, id, o) + if err != nil { return err } m.emitNotification(ctx, intent) return nil } +func (m *Manager) notificationIntentForCurrentSCM(ctx context.Context, id domain.SessionID, o ports.SCMObservation) (*ports.NotificationIntent, error) { + // Serialize the session snapshot with activity transitions so ready-to-merge + // notifications do not race against a simultaneous waiting_input update. + m.mu.Lock() + defer m.mu.Unlock() + rec, ok, err := m.store.GetSession(ctx, id) + if err != nil { + return nil, err + } + if !ok { + return nil, fmt.Errorf("%w: %s", ports.ErrSessionNotFound, id) + } + return m.notificationIntentForSCM(rec, o), nil +} + func (m *Manager) notificationIntentForSCM(rec domain.SessionRecord, o ports.SCMObservation) *ports.NotificationIntent { prURL := firstSCMNonEmpty(o.PR.URL, o.PR.HTMLURL) base := ports.NotificationIntent{ @@ -123,7 +138,7 @@ func (m *Manager) notificationIntentForSCM(rec domain.SessionRecord, o ports.SCM base.Type = domain.NotificationPRMerged return &base } - if o.PR.Closed && !o.PR.Merged { + if o.PR.Closed { base.Type = domain.NotificationPRClosedUnmerged return &base } diff --git a/backend/internal/ports/session.go b/backend/internal/ports/session.go index 14742ea4..0c28f179 100644 --- a/backend/internal/ports/session.go +++ b/backend/internal/ports/session.go @@ -1,6 +1,13 @@ package ports -import "github.com/aoagents/agent-orchestrator/backend/internal/domain" +import ( + "errors" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +// ErrSessionNotFound reports an observation for an unknown session id. +var ErrSessionNotFound = errors.New("session not found") // SpawnConfig is the request to start a new session: which project/issue, which // agent harness, and the branch/prompt the agent launches with. diff --git a/backend/internal/storage/sqlite/gen/notifications.sql.go b/backend/internal/storage/sqlite/gen/notifications.sql.go index 86036e6f..0fa6b4e1 100644 --- a/backend/internal/storage/sqlite/gen/notifications.sql.go +++ b/backend/internal/storage/sqlite/gen/notifications.sql.go @@ -58,6 +58,36 @@ func (q *Queries) CreateNotification(ctx context.Context, arg CreateNotification return i, err } +const getUnreadNotificationByDedupe = `-- name: GetUnreadNotificationByDedupe :one +SELECT id, session_id, project_id, pr_url, type, title, body, status, created_at +FROM notifications +WHERE session_id = ? AND type = ? AND pr_url = ? AND status = 'unread' +LIMIT 1 +` + +type GetUnreadNotificationByDedupeParams struct { + SessionID domain.SessionID + Type domain.NotificationType + PRURL string +} + +func (q *Queries) GetUnreadNotificationByDedupe(ctx context.Context, arg GetUnreadNotificationByDedupeParams) (Notification, error) { + row := q.db.QueryRowContext(ctx, getUnreadNotificationByDedupe, arg.SessionID, arg.Type, arg.PRURL) + var i Notification + err := row.Scan( + &i.ID, + &i.SessionID, + &i.ProjectID, + &i.PRURL, + &i.Type, + &i.Title, + &i.Body, + &i.Status, + &i.CreatedAt, + ) + return i, err +} + const listUnreadNotifications = `-- name: ListUnreadNotifications :many SELECT id, session_id, project_id, pr_url, type, title, body, status, created_at FROM notifications diff --git a/backend/internal/storage/sqlite/queries/notifications.sql b/backend/internal/storage/sqlite/queries/notifications.sql index 8a8e0e44..161146d9 100644 --- a/backend/internal/storage/sqlite/queries/notifications.sql +++ b/backend/internal/storage/sqlite/queries/notifications.sql @@ -17,3 +17,9 @@ FROM notifications WHERE project_id = ? AND status = 'unread' ORDER BY created_at DESC LIMIT ?; + +-- name: GetUnreadNotificationByDedupe :one +SELECT * +FROM notifications +WHERE session_id = ? AND type = ? AND pr_url = ? AND status = 'unread' +LIMIT 1; diff --git a/backend/internal/storage/sqlite/store/notification_store.go b/backend/internal/storage/sqlite/store/notification_store.go index 68b7682e..b19481e2 100644 --- a/backend/internal/storage/sqlite/store/notification_store.go +++ b/backend/internal/storage/sqlite/store/notification_store.go @@ -2,9 +2,9 @@ package store import ( "context" + "database/sql" "errors" "fmt" - "strings" moderncsqlite "modernc.org/sqlite" sqlite3 "modernc.org/sqlite/lib" @@ -24,6 +24,11 @@ func (s *Store) CreateNotification(ctx context.Context, rec domain.NotificationR } s.writeMu.Lock() defer s.writeMu.Unlock() + if existing, ok, err := s.getUnreadNotificationByDedupe(ctx, rec); err != nil { + return domain.NotificationRecord{}, false, err + } else if ok { + return existing, false, nil + } row, err := s.qw.CreateNotification(ctx, gen.CreateNotificationParams{ ID: rec.ID, SessionID: rec.SessionID, @@ -36,8 +41,12 @@ func (s *Store) CreateNotification(ctx context.Context, rec domain.NotificationR CreatedAt: rec.CreatedAt, }) if err != nil { - if isUnreadNotificationDuplicate(err) { - return domain.NotificationRecord{}, false, nil + if isSQLiteUnique(err) { + if existing, ok, lookupErr := s.getUnreadNotificationByDedupe(ctx, rec); lookupErr != nil { + return domain.NotificationRecord{}, false, lookupErr + } else if ok { + return existing, false, nil + } } return domain.NotificationRecord{}, false, fmt.Errorf("create notification %s: %w", rec.ID, err) } @@ -62,15 +71,24 @@ func (s *Store) ListUnreadNotificationsByProject(ctx context.Context, projectID return notificationsFromGen(rows), nil } -func isUnreadNotificationDuplicate(err error) bool { - var sqliteErr *moderncsqlite.Error - if !errors.As(err, &sqliteErr) || sqliteErr.Code() != sqlite3.SQLITE_CONSTRAINT_UNIQUE { - return false +func (s *Store) getUnreadNotificationByDedupe(ctx context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) { + row, err := s.qw.GetUnreadNotificationByDedupe(ctx, gen.GetUnreadNotificationByDedupeParams{ + SessionID: rec.SessionID, + Type: rec.Type, + PRURL: rec.PRURL, + }) + if errors.Is(err, sql.ErrNoRows) { + return domain.NotificationRecord{}, false, nil } - msg := err.Error() - return strings.Contains(msg, "notifications.session_id") && - strings.Contains(msg, "notifications.type") && - strings.Contains(msg, "notifications.pr_url") + if err != nil { + return domain.NotificationRecord{}, false, fmt.Errorf("lookup unread notification dedupe: %w", err) + } + return notificationFromGen(row), true, nil +} + +func isSQLiteUnique(err error) bool { + var sqliteErr *moderncsqlite.Error + return errors.As(err, &sqliteErr) && sqliteErr.Code() == sqlite3.SQLITE_CONSTRAINT_UNIQUE } func notificationFromGen(row gen.Notification) domain.NotificationRecord { From 5634c1b4379376118ff1d5f7723cb7fc88623ec6 Mon Sep 17 00:00:00 2001 From: whoisasx Date: Thu, 11 Jun 2026 19:42:57 +0530 Subject: [PATCH 3/6] fix: require passing CI for merge-ready notifications --- backend/internal/lifecycle/manager_test.go | 3 +++ backend/internal/lifecycle/reactions.go | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/backend/internal/lifecycle/manager_test.go b/backend/internal/lifecycle/manager_test.go index b43f2539..04657dce 100644 --- a/backend/internal/lifecycle/manager_test.go +++ b/backend/internal/lifecycle/manager_test.go @@ -657,6 +657,9 @@ func TestSCMObservation_Notifications(t *testing.T) { func TestSCMObservation_NotReadyWhenCIOrReviewBlocks(t *testing.T) { for _, obs := range []ports.SCMObservation{ {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIFailing)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, + {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIPending)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, + {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIUnknown)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, + {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIPassing)}, Review: ports.SCMReviewObservation{Decision: string(domain.ReviewChangesRequest)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, } { st := newFakeStore() diff --git a/backend/internal/lifecycle/reactions.go b/backend/internal/lifecycle/reactions.go index ad8ef8ac..2519087d 100644 --- a/backend/internal/lifecycle/reactions.go +++ b/backend/internal/lifecycle/reactions.go @@ -153,7 +153,12 @@ func scmObservationIsReadyToMerge(o ports.SCMObservation) bool { if o.PR.Merged || o.PR.Closed || o.PR.Draft { return false } - if domain.CIState(o.CI.Summary) == domain.CIFailing { + ci := domain.CIState(o.CI.Summary) + if ci == "" { + ci = domain.CIUnknown + } + switch ci { + case domain.CIFailing, domain.CIPending, domain.CIUnknown: return false } if domain.ReviewDecision(o.Review.Decision) == domain.ReviewChangesRequest || hasUnresolvedSCMComments(o.Review.Threads) { From 2dcd34869c8c0b2f716b18e531a9cb60daaaf91d Mon Sep 17 00:00:00 2001 From: whoisasx Date: Fri, 12 Jun 2026 16:08:25 +0530 Subject: [PATCH 4/6] fix: simplify notification listing --- backend/internal/daemon/daemon.go | 5 +- backend/internal/httpd/apispec/openapi.yaml | 6 --- backend/internal/httpd/controllers/dto.go | 5 +- .../httpd/controllers/notifications.go | 7 ++- .../httpd/controllers/notifications_test.go | 4 +- .../service/notification/dispatcher.go | 32 ------------ .../internal/service/notification/service.go | 40 +++++---------- .../service/notification/service_test.go | 51 +++---------------- .../internal/service/notification/store.go | 1 - .../internal/service/notification/types.go | 3 +- .../storage/sqlite/gen/notifications.sql.go | 46 ----------------- .../sqlite/migrations/0011_notifications.sql | 14 ++--- .../storage/sqlite/queries/notifications.sql | 6 --- .../sqlite/store/notification_store.go | 9 ---- .../sqlite/store/notification_store_test.go | 10 ++-- frontend/src/api/schema.ts | 2 - 16 files changed, 37 insertions(+), 204 deletions(-) delete mode 100644 backend/internal/service/notification/dispatcher.go diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index 58dae216..fb4674e1 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -83,10 +83,7 @@ func Run() error { // Built before the Lifecycle Manager so the LCM can use it for SCM-driven // agent nudges (CI failure, review feedback, merge conflict). messenger := newSessionMessenger(store, runtimeAdapter, log) - notifier := notificationsvc.New(notificationsvc.Deps{ - Store: store, - Dispatcher: notificationsvc.NewDashboardDispatcher(nil), - }) + notifier := notificationsvc.New(notificationsvc.Deps{Store: store}) // Bring up the Lifecycle Manager and the reaper first: it makes the session // lifecycle write path live (reducer write -> store -> DB trigger -> diff --git a/backend/internal/httpd/apispec/openapi.yaml b/backend/internal/httpd/apispec/openapi.yaml index 45f95fde..e98c9d24 100644 --- a/backend/internal/httpd/apispec/openapi.yaml +++ b/backend/internal/httpd/apispec/openapi.yaml @@ -63,12 +63,6 @@ paths: enum: - unread type: string - - description: Optional project id filter. - in: query - name: projectId - schema: - description: Optional project id filter. - type: string - description: Maximum notifications to return. Defaults to 50; capped at 100. in: query name: limit diff --git a/backend/internal/httpd/controllers/dto.go b/backend/internal/httpd/controllers/dto.go index 2a03d65d..f4c872d2 100644 --- a/backend/internal/httpd/controllers/dto.go +++ b/backend/internal/httpd/controllers/dto.go @@ -264,9 +264,8 @@ type OrchestratorResponse struct { // ListNotificationsQuery is the query string accepted by GET /api/v1/notifications. type ListNotificationsQuery struct { - Status string `query:"status,omitempty" enum:"unread" description:"Notification status filter. V1 supports only unread."` - ProjectID string `query:"projectId,omitempty" description:"Optional project id filter."` - Limit int `query:"limit,omitempty" minimum:"1" maximum:"100" description:"Maximum notifications to return. Defaults to 50; capped at 100."` + Status string `query:"status,omitempty" enum:"unread" description:"Notification status filter. V1 supports only unread."` + Limit int `query:"limit,omitempty" minimum:"1" maximum:"100" description:"Maximum notifications to return. Defaults to 50; capped at 100."` } // NotificationTarget is the dashboard navigation target for a notification. diff --git a/backend/internal/httpd/controllers/notifications.go b/backend/internal/httpd/controllers/notifications.go index ac9ea934..54bb1e08 100644 --- a/backend/internal/httpd/controllers/notifications.go +++ b/backend/internal/httpd/controllers/notifications.go @@ -7,7 +7,6 @@ import ( "github.com/go-chi/chi/v5" - "github.com/aoagents/agent-orchestrator/backend/internal/domain" "github.com/aoagents/agent-orchestrator/backend/internal/httpd/apispec" "github.com/aoagents/agent-orchestrator/backend/internal/httpd/envelope" notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification" @@ -50,9 +49,9 @@ func parseNotificationListFilter(r *http.Request) (notificationsvc.ListFilter, e q := r.URL.Query() status := q.Get("status") if status == "" { - status = string(domain.NotificationUnread) + status = "unread" } - if status != string(domain.NotificationUnread) { + if status != "unread" { return notificationsvc.ListFilter{}, errNotificationStatusUnsupported } limit := notificationsvc.DefaultListLimit @@ -66,7 +65,7 @@ func parseNotificationListFilter(r *http.Request) (notificationsvc.ListFilter, e if limit > notificationsvc.MaxListLimit { limit = notificationsvc.MaxListLimit } - return notificationsvc.ListFilter{ProjectID: domain.ProjectID(q.Get("projectId")), Limit: limit}, nil + return notificationsvc.ListFilter{Limit: limit}, nil } var ( diff --git a/backend/internal/httpd/controllers/notifications_test.go b/backend/internal/httpd/controllers/notifications_test.go index 7323a12b..61e1121c 100644 --- a/backend/internal/httpd/controllers/notifications_test.go +++ b/backend/internal/httpd/controllers/notifications_test.go @@ -43,11 +43,11 @@ func TestNotificationsAPI_ListUnread(t *testing.T) { }}} srv := newNotificationTestServer(t, svc) - body, status, _ := doRequest(t, srv, "GET", "/api/v1/notifications?projectId=mer&limit=10", "") + body, status, _ := doRequest(t, srv, "GET", "/api/v1/notifications?limit=10", "") if status != http.StatusOK { t.Fatalf("status = %d, want 200; body=%s", status, body) } - if svc.gotFilter.ProjectID != "mer" || svc.gotFilter.Limit != 10 { + if svc.gotFilter.Limit != 10 { t.Fatalf("filter = %+v", svc.gotFilter) } var resp struct { diff --git a/backend/internal/service/notification/dispatcher.go b/backend/internal/service/notification/dispatcher.go deleted file mode 100644 index 32ca518e..00000000 --- a/backend/internal/service/notification/dispatcher.go +++ /dev/null @@ -1,32 +0,0 @@ -package notification - -import "context" - -// Dispatcher is the deliberately tiny v1 delivery boundary. -type Dispatcher interface { - Dispatch(ctx context.Context, n Notification) error -} - -// DashboardPublisher publishes created notifications to dashboard transports. -type DashboardPublisher interface { - Publish(ctx context.Context, n Notification) error -} - -// DashboardDispatcher forwards created notifications to a dashboard publisher. -type DashboardDispatcher struct { - publisher DashboardPublisher -} - -// NewDashboardDispatcher builds a dispatcher around publisher. A nil publisher -// makes dispatch a no-op, which is the v1 daemon default until SSE is added. -func NewDashboardDispatcher(publisher DashboardPublisher) DashboardDispatcher { - return DashboardDispatcher{publisher: publisher} -} - -// Dispatch sends n to the dashboard publisher when one is configured. -func (d DashboardDispatcher) Dispatch(ctx context.Context, n Notification) error { - if d.publisher == nil { - return nil - } - return d.publisher.Publish(ctx, n) -} diff --git a/backend/internal/service/notification/service.go b/backend/internal/service/notification/service.go index ac5099fe..29ab980a 100644 --- a/backend/internal/service/notification/service.go +++ b/backend/internal/service/notification/service.go @@ -18,26 +18,24 @@ const ( MaxListLimit = 100 ) -// Manager validates lifecycle intents, enriches them into stored rows, persists -// unread notifications, and dispatches newly created rows. +// Manager validates lifecycle intents, enriches them into stored rows, and persists +// unread notifications. type Manager struct { - store Store - dispatcher Dispatcher - clock func() time.Time - newID func() string + store Store + clock func() time.Time + newID func() string } // Deps configures a Manager. type Deps struct { - Store Store - Dispatcher Dispatcher - Clock func() time.Time - NewID func() string + Store Store + Clock func() time.Time + NewID func() string } // New constructs a Manager with production defaults for optional collaborators. func New(d Deps) *Manager { - m := &Manager{store: d.Store, dispatcher: d.Dispatcher, clock: d.Clock, newID: d.NewID} + m := &Manager{store: d.Store, clock: d.Clock, newID: d.NewID} if m.clock == nil { m.clock = time.Now } @@ -47,8 +45,7 @@ func New(d Deps) *Manager { return m } -// Notify stores and dispatches one notification intent. Duplicate unread rows -// are treated as a clean no-op. +// Notify stores one notification intent. Duplicate unread rows are treated as a clean no-op. func (m *Manager) Notify(ctx context.Context, intent Intent) error { if m == nil || m.store == nil { return errors.New("notification: store is required") @@ -61,32 +58,23 @@ func (m *Manager) Notify(ctx context.Context, intent Intent) error { return fmt.Errorf("notification enrich: %w", err) } rec.ID = m.newID() - created, inserted, err := m.store.CreateNotification(ctx, rec) + _, inserted, err := m.store.CreateNotification(ctx, rec) if err != nil { return fmt.Errorf("notification store: %w", err) } if !inserted { return nil } - if m.dispatcher == nil { - return nil - } - return m.dispatcher.Dispatch(ctx, notificationFromRecord(created)) + return nil } -// ListUnread returns unread notifications newest-first, optionally limited to a project. +// ListUnread returns unread notifications newest-first. func (m *Manager) ListUnread(ctx context.Context, filter ListFilter) ([]Notification, error) { if m == nil || m.store == nil { return nil, errors.New("notification: store is required") } limit := normalizeLimit(filter.Limit) - var rows []domain.NotificationRecord - var err error - if filter.ProjectID != "" { - rows, err = m.store.ListUnreadNotificationsByProject(ctx, filter.ProjectID, limit) - } else { - rows, err = m.store.ListUnreadNotifications(ctx, limit) - } + rows, err := m.store.ListUnreadNotifications(ctx, limit) if err != nil { return nil, err } diff --git a/backend/internal/service/notification/service_test.go b/backend/internal/service/notification/service_test.go index a676b7c2..1b2fc52b 100644 --- a/backend/internal/service/notification/service_test.go +++ b/backend/internal/service/notification/service_test.go @@ -30,31 +30,10 @@ func (f *fakeStore) ListUnreadNotifications(_ context.Context, _ int) ([]domain. return f.rows, nil } -func (f *fakeStore) ListUnreadNotificationsByProject(_ context.Context, projectID domain.ProjectID, _ int) ([]domain.NotificationRecord, error) { - out := make([]domain.NotificationRecord, 0, len(f.rows)) - for _, row := range f.rows { - if row.ProjectID == projectID { - out = append(out, row) - } - } - return out, nil -} - -type captureDispatcher struct { - notifications []Notification - err error -} - -func (d *captureDispatcher) Dispatch(_ context.Context, n Notification) error { - d.notifications = append(d.notifications, n) - return d.err -} - -func TestManagerNotifyPersistsThenDispatches(t *testing.T) { +func TestManagerNotifyPersistsNotification(t *testing.T) { st := &fakeStore{} - dispatch := &captureDispatcher{} now := time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC) - mgr := New(Deps{Store: st, Dispatcher: dispatch, Clock: func() time.Time { return now }, NewID: func() string { return "ntf_1" }}) + mgr := New(Deps{Store: st, Clock: func() time.Time { return now }, NewID: func() string { return "ntf_1" }}) if err := mgr.Notify(context.Background(), Intent{ Type: domain.NotificationNeedsInput, @@ -70,36 +49,18 @@ func TestManagerNotifyPersistsThenDispatches(t *testing.T) { if got := st.rows[0]; got.ID != "ntf_1" || got.CreatedAt != now || got.Status != domain.NotificationUnread || got.Title != "checkout-flow needs input" { t.Fatalf("stored notification = %+v", got) } - if len(dispatch.notifications) != 1 || dispatch.notifications[0].ID != "ntf_1" { - t.Fatalf("dispatch = %+v", dispatch.notifications) - } } -func TestManagerNotifyDuplicateDoesNotDispatch(t *testing.T) { +func TestManagerNotifyDuplicateIsIgnored(t *testing.T) { st := &fakeStore{duplicate: true} - dispatch := &captureDispatcher{} - mgr := New(Deps{Store: st, Dispatcher: dispatch, Clock: func() time.Time { return time.Now() }, NewID: func() string { return "ntf_1" }}) + mgr := New(Deps{Store: st, Clock: func() time.Time { return time.Now() }, NewID: func() string { return "ntf_1" }}) err := mgr.Notify(context.Background(), Intent{Type: domain.NotificationNeedsInput, SessionID: "mer-1", ProjectID: "mer", CreatedAt: time.Now()}) if err != nil { t.Fatalf("Notify duplicate: %v", err) } - if len(dispatch.notifications) != 0 { - t.Fatalf("duplicate should not dispatch, got %+v", dispatch.notifications) - } -} - -func TestManagerNotifyDispatchFailureLeavesStoredRow(t *testing.T) { - st := &fakeStore{} - dispatch := &captureDispatcher{err: errors.New("sse down")} - mgr := New(Deps{Store: st, Dispatcher: dispatch, Clock: func() time.Time { return time.Now() }, NewID: func() string { return "ntf_1" }}) - - err := mgr.Notify(context.Background(), Intent{Type: domain.NotificationNeedsInput, SessionID: "mer-1", ProjectID: "mer", CreatedAt: time.Now()}) - if err == nil { - t.Fatal("want dispatch error") - } - if len(st.rows) != 1 { - t.Fatalf("row should remain stored after dispatch failure; rows=%d", len(st.rows)) + if len(st.rows) != 0 { + t.Fatalf("duplicate should not persist, got %+v", st.rows) } } diff --git a/backend/internal/service/notification/store.go b/backend/internal/service/notification/store.go index 296e6aec..022def81 100644 --- a/backend/internal/service/notification/store.go +++ b/backend/internal/service/notification/store.go @@ -10,5 +10,4 @@ import ( type Store interface { CreateNotification(ctx context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) ListUnreadNotifications(ctx context.Context, limit int) ([]domain.NotificationRecord, error) - ListUnreadNotificationsByProject(ctx context.Context, projectID domain.ProjectID, limit int) ([]domain.NotificationRecord, error) } diff --git a/backend/internal/service/notification/types.go b/backend/internal/service/notification/types.go index 705c6d4d..c1c6d4c2 100644 --- a/backend/internal/service/notification/types.go +++ b/backend/internal/service/notification/types.go @@ -36,6 +36,5 @@ type Notification struct { // ListFilter controls unread notification listing. type ListFilter struct { - ProjectID domain.ProjectID - Limit int + Limit int } diff --git a/backend/internal/storage/sqlite/gen/notifications.sql.go b/backend/internal/storage/sqlite/gen/notifications.sql.go index 0fa6b4e1..a70c6df5 100644 --- a/backend/internal/storage/sqlite/gen/notifications.sql.go +++ b/backend/internal/storage/sqlite/gen/notifications.sql.go @@ -128,49 +128,3 @@ func (q *Queries) ListUnreadNotifications(ctx context.Context, limit int64) ([]N } return items, nil } - -const listUnreadNotificationsByProject = `-- name: ListUnreadNotificationsByProject :many -SELECT id, session_id, project_id, pr_url, type, title, body, status, created_at -FROM notifications -WHERE project_id = ? AND status = 'unread' -ORDER BY created_at DESC -LIMIT ? -` - -type ListUnreadNotificationsByProjectParams struct { - ProjectID domain.ProjectID - Limit int64 -} - -func (q *Queries) ListUnreadNotificationsByProject(ctx context.Context, arg ListUnreadNotificationsByProjectParams) ([]Notification, error) { - rows, err := q.db.QueryContext(ctx, listUnreadNotificationsByProject, arg.ProjectID, arg.Limit) - if err != nil { - return nil, err - } - defer rows.Close() - items := []Notification{} - for rows.Next() { - var i Notification - if err := rows.Scan( - &i.ID, - &i.SessionID, - &i.ProjectID, - &i.PRURL, - &i.Type, - &i.Title, - &i.Body, - &i.Status, - &i.CreatedAt, - ); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} diff --git a/backend/internal/storage/sqlite/migrations/0011_notifications.sql b/backend/internal/storage/sqlite/migrations/0011_notifications.sql index 0612952b..cd12d7d0 100644 --- a/backend/internal/storage/sqlite/migrations/0011_notifications.sql +++ b/backend/internal/storage/sqlite/migrations/0011_notifications.sql @@ -15,19 +15,13 @@ CREATE TABLE notifications ( ), title TEXT NOT NULL, body TEXT NOT NULL DEFAULT '', - status TEXT NOT NULL DEFAULT 'unread' CHECK (status IN ('read', 'unread')), + status TEXT NOT NULL DEFAULT 'unread', created_at TIMESTAMP NOT NULL ); -CREATE INDEX idx_notifications_unread +CREATE INDEX idx_notifications_status ON notifications(status, created_at DESC); -CREATE INDEX idx_notifications_project_unread - ON notifications(project_id, status, created_at DESC); - -CREATE INDEX idx_notifications_session - ON notifications(session_id, created_at DESC); - CREATE UNIQUE INDEX idx_notifications_unread_dedupe ON notifications(session_id, type, pr_url) WHERE status = 'unread'; @@ -36,8 +30,6 @@ CREATE UNIQUE INDEX idx_notifications_unread_dedupe -- +goose Down -- +goose StatementBegin DROP INDEX IF EXISTS idx_notifications_unread_dedupe; -DROP INDEX IF EXISTS idx_notifications_session; -DROP INDEX IF EXISTS idx_notifications_project_unread; -DROP INDEX IF EXISTS idx_notifications_unread; +DROP INDEX IF EXISTS idx_notifications_status; DROP TABLE IF EXISTS notifications; -- +goose StatementEnd diff --git a/backend/internal/storage/sqlite/queries/notifications.sql b/backend/internal/storage/sqlite/queries/notifications.sql index 161146d9..bd8a75b5 100644 --- a/backend/internal/storage/sqlite/queries/notifications.sql +++ b/backend/internal/storage/sqlite/queries/notifications.sql @@ -11,12 +11,6 @@ WHERE status = 'unread' ORDER BY created_at DESC LIMIT ?; --- name: ListUnreadNotificationsByProject :many -SELECT * -FROM notifications -WHERE project_id = ? AND status = 'unread' -ORDER BY created_at DESC -LIMIT ?; -- name: GetUnreadNotificationByDedupe :one SELECT * diff --git a/backend/internal/storage/sqlite/store/notification_store.go b/backend/internal/storage/sqlite/store/notification_store.go index b19481e2..ce54293f 100644 --- a/backend/internal/storage/sqlite/store/notification_store.go +++ b/backend/internal/storage/sqlite/store/notification_store.go @@ -62,15 +62,6 @@ func (s *Store) ListUnreadNotifications(ctx context.Context, limit int) ([]domai return notificationsFromGen(rows), nil } -// ListUnreadNotificationsByProject returns unread notifications for one project newest-first. -func (s *Store) ListUnreadNotificationsByProject(ctx context.Context, projectID domain.ProjectID, limit int) ([]domain.NotificationRecord, error) { - rows, err := s.qr.ListUnreadNotificationsByProject(ctx, gen.ListUnreadNotificationsByProjectParams{ProjectID: projectID, Limit: int64(limit)}) - if err != nil { - return nil, fmt.Errorf("list unread notifications for %s: %w", projectID, err) - } - return notificationsFromGen(rows), nil -} - func (s *Store) getUnreadNotificationByDedupe(ctx context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) { row, err := s.qw.GetUnreadNotificationByDedupe(ctx, gen.GetUnreadNotificationByDedupeParams{ SessionID: rec.SessionID, diff --git a/backend/internal/storage/sqlite/store/notification_store_test.go b/backend/internal/storage/sqlite/store/notification_store_test.go index 77c02674..0b2ebf9b 100644 --- a/backend/internal/storage/sqlite/store/notification_store_test.go +++ b/backend/internal/storage/sqlite/store/notification_store_test.go @@ -49,7 +49,7 @@ func TestNotificationStore_InsertListAndDedupe(t *testing.T) { } } -func TestNotificationStore_ProjectFilterAndNewestFirst(t *testing.T) { +func TestNotificationStore_ListUnreadNewestFirstAcrossProjects(t *testing.T) { s := newTestStore(t) ctx := context.Background() seedProject(t, s, "mer") @@ -66,12 +66,12 @@ func TestNotificationStore_ProjectFilterAndNewestFirst(t *testing.T) { t.Fatalf("insert %s inserted=%v err=%v", rec.ID, inserted, err) } } - rows, err := s.ListUnreadNotificationsByProject(ctx, "mer", 10) + rows, err := s.ListUnreadNotifications(ctx, 2) if err != nil { - t.Fatalf("ListUnreadNotificationsByProject: %v", err) + t.Fatalf("ListUnreadNotifications: %v", err) } - if len(rows) != 2 || rows[0].ID != "new" || rows[1].ID != "old" { - t.Fatalf("project rows = %+v", rows) + if len(rows) != 2 || rows[0].ID != "other" || rows[1].ID != "new" { + t.Fatalf("rows = %+v", rows) } } diff --git a/frontend/src/api/schema.ts b/frontend/src/api/schema.ts index 3fe5cd03..00fa0830 100644 --- a/frontend/src/api/schema.ts +++ b/frontend/src/api/schema.ts @@ -724,8 +724,6 @@ export interface operations { query?: { /** @description Notification status filter. V1 supports only unread. */ status?: "unread"; - /** @description Optional project id filter. */ - projectId?: string; /** @description Maximum notifications to return. Defaults to 50; capped at 100. */ limit?: number; }; From c38895e8445dab4377536eda93cb5e221f3289c9 Mon Sep 17 00:00:00 2001 From: whoisasx Date: Fri, 12 Jun 2026 16:19:03 +0530 Subject: [PATCH 5/6] fix: ignore missing sessions for scm notifications --- backend/internal/lifecycle/manager_test.go | 16 ++++++++++++++++ backend/internal/lifecycle/reactions.go | 3 +-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/backend/internal/lifecycle/manager_test.go b/backend/internal/lifecycle/manager_test.go index 04657dce..56e4cf11 100644 --- a/backend/internal/lifecycle/manager_test.go +++ b/backend/internal/lifecycle/manager_test.go @@ -192,6 +192,22 @@ func TestSCMObservationProjectsToExistingPRReactions(t *testing.T) { } } +func TestSCMObservation_MissingSessionIsIgnored(t *testing.T) { + st := newFakeStore() + sink := &fakeNotificationSink{} + m := New(st, nil, WithNotificationSink(sink)) + o := ports.SCMObservation{ + Fetched: true, + PR: ports.SCMPRObservation{URL: "pr1", Number: 1}, + } + if err := m.ApplySCMObservation(ctx, "missing-1", o); err != nil { + t.Fatalf("ApplySCMObservation missing session: %v", err) + } + if len(sink.intents) != 0 { + t.Fatalf("missing session emitted notifications: %+v", sink.intents) + } +} + func TestSCMObservationUsesPRHeadWhenCIHeadMissing(t *testing.T) { m, st, msg := newManager() st.sessions["mer-1"] = working("mer-1") diff --git a/backend/internal/lifecycle/reactions.go b/backend/internal/lifecycle/reactions.go index 2519087d..bfffd075 100644 --- a/backend/internal/lifecycle/reactions.go +++ b/backend/internal/lifecycle/reactions.go @@ -3,7 +3,6 @@ package lifecycle import ( "context" "encoding/json" - "fmt" "log/slog" "strings" "sync" @@ -114,7 +113,7 @@ func (m *Manager) notificationIntentForCurrentSCM(ctx context.Context, id domain return nil, err } if !ok { - return nil, fmt.Errorf("%w: %s", ports.ErrSessionNotFound, id) + return nil, nil } return m.notificationIntentForSCM(rec, o), nil } From aadfe52e87de6de2369a79d76001c8ac367a9e3c Mon Sep 17 00:00:00 2001 From: whoisasx Date: Fri, 12 Jun 2026 19:04:58 +0530 Subject: [PATCH 6/6] fix: project notifications from cdc --- backend/internal/cdc/event.go | 1 + backend/internal/daemon/daemon.go | 11 +- backend/internal/daemon/lifecycle_wiring.go | 8 +- backend/internal/daemon/wiring_test.go | 2 +- backend/internal/lifecycle/manager.go | 47 +-- backend/internal/lifecycle/manager_test.go | 147 +------ backend/internal/lifecycle/reactions.go | 87 +--- .../internal/observe/notification/enrich.go | 107 +++++ .../observe/notification/projector.go | 253 ++++++++++++ .../observe/notification/projector_test.go | 121 ++++++ backend/internal/ports/notifications.go | 27 -- .../internal/service/notification/enrich.go | 90 ----- .../internal/service/notification/service.go | 45 +-- .../service/notification/service_test.go | 68 +--- .../internal/service/notification/store.go | 3 +- .../internal/service/notification/types.go | 12 +- .../sqlite/migrations/0011_notifications.sql | 378 +++++++++++++++++- .../sqlite/store/notification_store_test.go | 41 ++ 18 files changed, 937 insertions(+), 511 deletions(-) create mode 100644 backend/internal/observe/notification/enrich.go create mode 100644 backend/internal/observe/notification/projector.go create mode 100644 backend/internal/observe/notification/projector_test.go delete mode 100644 backend/internal/ports/notifications.go delete mode 100644 backend/internal/service/notification/enrich.go diff --git a/backend/internal/cdc/event.go b/backend/internal/cdc/event.go index 4f43d8f2..04999421 100644 --- a/backend/internal/cdc/event.go +++ b/backend/internal/cdc/event.go @@ -28,6 +28,7 @@ const ( EventPRSessionChanged EventType = "pr_session_changed" EventPRReviewThreadAdded EventType = "pr_review_thread_added" EventPRReviewThreadResolved EventType = "pr_review_thread_resolved" + EventNotificationCreated EventType = "notification_created" ) // Event is one CDC change read from change_log. Seq is the monotonic ordering + diff --git a/backend/internal/daemon/daemon.go b/backend/internal/daemon/daemon.go index fb4674e1..5ea8d320 100644 --- a/backend/internal/daemon/daemon.go +++ b/backend/internal/daemon/daemon.go @@ -14,6 +14,7 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/adapters/runtime/zellij" "github.com/aoagents/agent-orchestrator/backend/internal/config" "github.com/aoagents/agent-orchestrator/backend/internal/httpd" + notificationproj "github.com/aoagents/agent-orchestrator/backend/internal/observe/notification" "github.com/aoagents/agent-orchestrator/backend/internal/runfile" notificationsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/notification" projectsvc "github.com/aoagents/agent-orchestrator/backend/internal/service/project" @@ -84,11 +85,16 @@ func Run() error { // agent nudges (CI failure, review feedback, merge conflict). messenger := newSessionMessenger(store, runtimeAdapter, log) notifier := notificationsvc.New(notificationsvc.Deps{Store: store}) + notificationDone := notificationproj.New(notificationproj.Deps{ + Store: store, + Live: cdcPipe.Broadcaster, + Logger: log, + }).Start(ctx) // Bring up the Lifecycle Manager and the reaper first: it makes the session // lifecycle write path live (reducer write -> store -> DB trigger -> // change_log -> poller -> broadcaster) and gives startSession the shared LCM. - lcStack := startLifecycle(ctx, store, runtimeAdapter, messenger, notifier, log) + lcStack := startLifecycle(ctx, store, runtimeAdapter, messenger, log) lcStack.scmDone = startSCMObserver(ctx, store, lcStack.LCM, log) // Wire the controller-facing session service over the same store + LCM, the @@ -99,6 +105,7 @@ func Run() error { if err != nil { stop() lcStack.Stop() + <-notificationDone if cdcErr := cdcPipe.Stop(); cdcErr != nil { log.Error("cdc pipeline shutdown", "err", cdcErr) } @@ -117,6 +124,7 @@ func Run() error { if err != nil { stop() lcStack.Stop() + <-notificationDone if cdcErr := cdcPipe.Stop(); cdcErr != nil { log.Error("cdc pipeline shutdown", "err", cdcErr) } @@ -131,6 +139,7 @@ func Run() error { // runs before the cancel — which would hang any non-signal exit path. stop() lcStack.Stop() + <-notificationDone if err := cdcPipe.Stop(); err != nil { log.Error("cdc pipeline shutdown", "err", err) } diff --git a/backend/internal/daemon/lifecycle_wiring.go b/backend/internal/daemon/lifecycle_wiring.go index c57e5062..b4d40905 100644 --- a/backend/internal/daemon/lifecycle_wiring.go +++ b/backend/internal/daemon/lifecycle_wiring.go @@ -20,10 +20,6 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/storage/sqlite" ) -type notificationSink interface { - Notify(context.Context, ports.NotificationIntent) error -} - // lifecycleStack owns the runtime reaper goroutine started with the lifecycle // reducer. The reducer itself is only used for wiring observations into storage. type lifecycleStack struct { @@ -39,8 +35,8 @@ type lifecycleStack struct { // reaper. The goroutine stops when ctx is cancelled; Stop waits for it to drain. // The messenger is the per-daemon agent messenger the LCM uses to nudge agents // in response to SCM observations (CI failure, review feedback, merge conflict). -func startLifecycle(ctx context.Context, store *sqlite.Store, runtime ports.Runtime, messenger ports.AgentMessenger, notifier notificationSink, logger *slog.Logger) *lifecycleStack { - lcm := lifecycle.New(store, messenger, lifecycle.WithNotificationSink(notifier)) +func startLifecycle(ctx context.Context, store *sqlite.Store, runtime ports.Runtime, messenger ports.AgentMessenger, logger *slog.Logger) *lifecycleStack { + lcm := lifecycle.New(store, messenger) rp := reaper.New(lcm, store, runtime, reaper.Config{Logger: logger}) return &lifecycleStack{LCM: lcm, reaperDone: rp.Start(ctx)} } diff --git a/backend/internal/daemon/wiring_test.go b/backend/internal/daemon/wiring_test.go index df194ad6..0e4815d0 100644 --- a/backend/internal/daemon/wiring_test.go +++ b/backend/internal/daemon/wiring_test.go @@ -315,7 +315,7 @@ func TestWiring_StartLifecycleThreadsMessengerIntoLCM(t *testing.T) { log := slog.New(slog.NewTextHandler(io.Discard, nil)) messenger := &captureMessenger{} - stack := startLifecycle(ctx, store, zellij.New(zellij.Options{}), messenger, nil, log) + stack := startLifecycle(ctx, store, zellij.New(zellij.Options{}), messenger, log) t.Cleanup(stack.Stop) t.Cleanup(cancel) diff --git a/backend/internal/lifecycle/manager.go b/backend/internal/lifecycle/manager.go index 738051a9..0fe43cda 100644 --- a/backend/internal/lifecycle/manager.go +++ b/backend/internal/lifecycle/manager.go @@ -7,7 +7,6 @@ package lifecycle import ( "context" "fmt" - "log/slog" "sync" "time" @@ -24,25 +23,11 @@ type sessionStore interface { UpdatePRLastNudgeSignature(ctx context.Context, prURL, payload string) error } -// notificationSink is the optional lifecycle-to-notification-service boundary. -type notificationSink interface { - Notify(ctx context.Context, intent ports.NotificationIntent) error -} - -// Option customizes a Manager. -type Option func(*Manager) - -// WithNotificationSink wires lifecycle notification intents to a sink. -func WithNotificationSink(sink notificationSink) Option { - return func(m *Manager) { m.notifications = sink } -} - // Manager reduces runtime, activity, spawn, and termination observations into durable session facts. // It also owns agent nudges caused by PR observations, including merge-conflict, CI-failure, and review-feedback prompts. type Manager struct { - store sessionStore - messenger ports.AgentMessenger - notifications notificationSink + store sessionStore + messenger ports.AgentMessenger mu sync.Mutex window time.Duration @@ -51,12 +36,8 @@ type Manager struct { } // New builds a Lifecycle Manager over the session store it writes and the messenger it uses for agent nudges. -func New(store sessionStore, messenger ports.AgentMessenger, opts ...Option) *Manager { - m := &Manager{store: store, messenger: messenger, window: defaultRecentActivityWindow, clock: time.Now, react: newReactionState()} - for _, opt := range opts { - opt(m) - } - return m +func New(store sessionStore, messenger ports.AgentMessenger) *Manager { + return &Manager{store: store, messenger: messenger, window: defaultRecentActivityWindow, clock: time.Now, react: newReactionState()} } func (m *Manager) mutate(ctx context.Context, id domain.SessionID, fn func(domain.SessionRecord, time.Time) (domain.SessionRecord, bool)) error { @@ -98,7 +79,6 @@ func (m *Manager) ApplyActivitySignal(ctx context.Context, id domain.SessionID, if !s.Valid { return nil } - var intent *ports.NotificationIntent m.mu.Lock() rec, ok, err := m.store.GetSession(ctx, id) if err != nil { @@ -137,29 +117,10 @@ func (m *Manager) ApplyActivitySignal(ctx context.Context, id domain.SessionID, m.mu.Unlock() return err } - if rec.Activity.State != domain.ActivityWaitingInput && next.Activity.State == domain.ActivityWaitingInput && !next.IsTerminated { - intent = &ports.NotificationIntent{ - Type: domain.NotificationNeedsInput, - SessionID: next.ID, - ProjectID: next.ProjectID, - CreatedAt: next.Activity.LastActivityAt, - SessionDisplayName: next.DisplayName, - } - } m.mu.Unlock() - m.emitNotification(ctx, intent) return nil } -func (m *Manager) emitNotification(ctx context.Context, intent *ports.NotificationIntent) { - if intent == nil || m.notifications == nil { - return - } - if err := m.notifications.Notify(ctx, *intent); err != nil { - slog.Default().Warn("lifecycle: notification failed", "session", intent.SessionID, "type", intent.Type, "err", err) - } -} - // MarkSpawned marks a newly spawned or restored session live and stores runtime/workspace handles. func (m *Manager) MarkSpawned(ctx context.Context, id domain.SessionID, metadata domain.SessionMetadata) error { m.mu.Lock() diff --git a/backend/internal/lifecycle/manager_test.go b/backend/internal/lifecycle/manager_test.go index 56e4cf11..ae0abb0c 100644 --- a/backend/internal/lifecycle/manager_test.go +++ b/backend/internal/lifecycle/manager_test.go @@ -194,8 +194,7 @@ func TestSCMObservationProjectsToExistingPRReactions(t *testing.T) { func TestSCMObservation_MissingSessionIsIgnored(t *testing.T) { st := newFakeStore() - sink := &fakeNotificationSink{} - m := New(st, nil, WithNotificationSink(sink)) + m := New(st, nil) o := ports.SCMObservation{ Fetched: true, PR: ports.SCMPRObservation{URL: "pr1", Number: 1}, @@ -203,9 +202,6 @@ func TestSCMObservation_MissingSessionIsIgnored(t *testing.T) { if err := m.ApplySCMObservation(ctx, "missing-1", o); err != nil { t.Fatalf("ApplySCMObservation missing session: %v", err) } - if len(sink.intents) != 0 { - t.Fatalf("missing session emitted notifications: %+v", sink.intents) - } } func TestSCMObservationUsesPRHeadWhenCIHeadMissing(t *testing.T) { @@ -570,144 +566,3 @@ func TestMarkSpawnedClearsFirstSignal(t *testing.T) { t.Fatalf("spawn/restore must clear the receipt, got %+v", got) } } - -type fakeNotificationSink struct { - intents []ports.NotificationIntent - err error -} - -func (f *fakeNotificationSink) Notify(_ context.Context, intent ports.NotificationIntent) error { - f.intents = append(f.intents, intent) - return f.err -} - -func TestActivity_WaitingInputTransitionEmitsNotification(t *testing.T) { - st := newFakeStore() - sink := &fakeNotificationSink{} - m := New(st, nil, WithNotificationSink(sink)) - now := time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC) - m.clock = func() time.Time { return now } - st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", DisplayName: "checkout-flow", Activity: domain.Activity{State: domain.ActivityActive, LastActivityAt: now.Add(-time.Minute)}, FirstSignalAt: now.Add(-time.Minute)} - - if err := m.ApplyActivitySignal(ctx, "mer-1", ports.ActivitySignal{Valid: true, State: domain.ActivityWaitingInput}); err != nil { - t.Fatal(err) - } - if len(sink.intents) != 1 { - t.Fatalf("intents = %d, want 1", len(sink.intents)) - } - intent := sink.intents[0] - if intent.Type != domain.NotificationNeedsInput || intent.SessionID != "mer-1" || intent.ProjectID != "mer" || intent.SessionDisplayName != "checkout-flow" { - t.Fatalf("intent = %+v", intent) - } -} - -func TestActivity_WaitingInputSameStateDoesNotEmitNotification(t *testing.T) { - st := newFakeStore() - sink := &fakeNotificationSink{} - m := New(st, nil, WithNotificationSink(sink)) - now := time.Now() - st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", Activity: domain.Activity{State: domain.ActivityWaitingInput, LastActivityAt: now}, FirstSignalAt: now} - - if err := m.ApplyActivitySignal(ctx, "mer-1", ports.ActivitySignal{Valid: true, State: domain.ActivityWaitingInput}); err != nil { - t.Fatal(err) - } - if len(sink.intents) != 0 { - t.Fatalf("same-state waiting_input emitted %+v", sink.intents) - } -} - -func TestActivity_TerminatedSessionDoesNotEmitNotification(t *testing.T) { - st := newFakeStore() - sink := &fakeNotificationSink{} - m := New(st, nil, WithNotificationSink(sink)) - st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", IsTerminated: true, Activity: domain.Activity{State: domain.ActivityExited}} - - if err := m.ApplyActivitySignal(ctx, "mer-1", ports.ActivitySignal{Valid: true, State: domain.ActivityWaitingInput}); err != nil { - t.Fatal(err) - } - if len(sink.intents) != 0 { - t.Fatalf("terminated session emitted %+v", sink.intents) - } -} - -func TestSCMObservation_Notifications(t *testing.T) { - for _, tc := range []struct { - name string - obs ports.SCMObservation - want domain.NotificationType - }{ - { - name: "ready", - obs: ports.SCMObservation{Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1, Title: "checkout"}, CI: ports.SCMCIObservation{Summary: string(domain.CIPassing)}, Review: ports.SCMReviewObservation{Decision: string(domain.ReviewApproved)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, - want: domain.NotificationReadyToMerge, - }, - { - name: "merged", - obs: ports.SCMObservation{Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/2", Number: 2, Merged: true}}, - want: domain.NotificationPRMerged, - }, - { - name: "closed", - obs: ports.SCMObservation{Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/3", Number: 3, Closed: true}}, - want: domain.NotificationPRClosedUnmerged, - }, - } { - t.Run(tc.name, func(t *testing.T) { - st := newFakeStore() - sink := &fakeNotificationSink{} - m := New(st, nil, WithNotificationSink(sink)) - st.sessions["mer-1"] = working("mer-1") - if err := m.ApplySCMObservation(ctx, "mer-1", tc.obs); err != nil { - t.Fatal(err) - } - if len(sink.intents) != 1 { - t.Fatalf("intents = %d, want 1", len(sink.intents)) - } - if got := sink.intents[0]; got.Type != tc.want || got.PRURL != tc.obs.PR.URL || got.PRNumber != tc.obs.PR.Number { - t.Fatalf("intent = %+v, want type %s", got, tc.want) - } - }) - } -} - -func TestSCMObservation_NotReadyWhenCIOrReviewBlocks(t *testing.T) { - for _, obs := range []ports.SCMObservation{ - {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIFailing)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, - {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIPending)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, - {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIUnknown)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, - {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, - {Fetched: true, PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, CI: ports.SCMCIObservation{Summary: string(domain.CIPassing)}, Review: ports.SCMReviewObservation{Decision: string(domain.ReviewChangesRequest)}, Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}}, - } { - st := newFakeStore() - sink := &fakeNotificationSink{} - m := New(st, nil, WithNotificationSink(sink)) - st.sessions["mer-1"] = working("mer-1") - if err := m.ApplySCMObservation(ctx, "mer-1", obs); err != nil { - t.Fatal(err) - } - if len(sink.intents) != 0 { - t.Fatalf("blocked PR emitted %+v", sink.intents) - } - } -} - -func TestSCMObservation_ReadyToMergeSuppressedWhileWaitingInput(t *testing.T) { - st := newFakeStore() - sink := &fakeNotificationSink{} - m := New(st, nil, WithNotificationSink(sink)) - rec := working("mer-1") - rec.Activity.State = domain.ActivityWaitingInput - st.sessions["mer-1"] = rec - obs := ports.SCMObservation{ - Fetched: true, - PR: ports.SCMPRObservation{URL: "https://github.com/o/r/pull/1", Number: 1}, - CI: ports.SCMCIObservation{Summary: string(domain.CIPassing)}, - Mergeability: ports.SCMMergeabilityObservation{State: string(domain.MergeMergeable)}, - } - if err := m.ApplySCMObservation(ctx, "mer-1", obs); err != nil { - t.Fatal(err) - } - if len(sink.intents) != 0 { - t.Fatalf("waiting-input session emitted ready notification: %+v", sink.intents) - } -} diff --git a/backend/internal/lifecycle/reactions.go b/backend/internal/lifecycle/reactions.go index bfffd075..614d3db9 100644 --- a/backend/internal/lifecycle/reactions.go +++ b/backend/internal/lifecycle/reactions.go @@ -92,92 +92,7 @@ func (m *Manager) ApplySCMObservation(ctx context.Context, id domain.SessionID, if !o.Fetched { return nil } - if err := m.ApplyPRObservation(ctx, id, scmToPRObservation(o)); err != nil { - return err - } - intent, err := m.notificationIntentForCurrentSCM(ctx, id, o) - if err != nil { - return err - } - m.emitNotification(ctx, intent) - return nil -} - -func (m *Manager) notificationIntentForCurrentSCM(ctx context.Context, id domain.SessionID, o ports.SCMObservation) (*ports.NotificationIntent, error) { - // Serialize the session snapshot with activity transitions so ready-to-merge - // notifications do not race against a simultaneous waiting_input update. - m.mu.Lock() - defer m.mu.Unlock() - rec, ok, err := m.store.GetSession(ctx, id) - if err != nil { - return nil, err - } - if !ok { - return nil, nil - } - return m.notificationIntentForSCM(rec, o), nil -} - -func (m *Manager) notificationIntentForSCM(rec domain.SessionRecord, o ports.SCMObservation) *ports.NotificationIntent { - prURL := firstSCMNonEmpty(o.PR.URL, o.PR.HTMLURL) - base := ports.NotificationIntent{ - SessionID: rec.ID, - ProjectID: rec.ProjectID, - PRURL: prURL, - CreatedAt: timeOr(o.ObservedAt, m.clock()), - SessionDisplayName: rec.DisplayName, - PRNumber: o.PR.Number, - PRTitle: o.PR.Title, - PRSourceBranch: o.PR.SourceBranch, - PRTargetBranch: o.PR.TargetBranch, - Provider: o.Provider, - Repo: o.Repo, - } - if o.PR.Merged { - base.Type = domain.NotificationPRMerged - return &base - } - if o.PR.Closed { - base.Type = domain.NotificationPRClosedUnmerged - return &base - } - if rec.IsTerminated || rec.Activity.State == domain.ActivityWaitingInput || !scmObservationIsReadyToMerge(o) { - return nil - } - base.Type = domain.NotificationReadyToMerge - return &base -} - -func scmObservationIsReadyToMerge(o ports.SCMObservation) bool { - if o.PR.Merged || o.PR.Closed || o.PR.Draft { - return false - } - ci := domain.CIState(o.CI.Summary) - if ci == "" { - ci = domain.CIUnknown - } - switch ci { - case domain.CIFailing, domain.CIPending, domain.CIUnknown: - return false - } - if domain.ReviewDecision(o.Review.Decision) == domain.ReviewChangesRequest || hasUnresolvedSCMComments(o.Review.Threads) { - return false - } - return domain.Mergeability(o.Mergeability.State) == domain.MergeMergeable -} - -func hasUnresolvedSCMComments(threads []ports.SCMReviewThreadObservation) bool { - for _, th := range threads { - if th.Resolved || th.IsBot { - continue - } - for _, c := range th.Comments { - if !c.IsBot { - return true - } - } - } - return false + return m.ApplyPRObservation(ctx, id, scmToPRObservation(o)) } func scmToPRObservation(o ports.SCMObservation) ports.PRObservation { diff --git a/backend/internal/observe/notification/enrich.go b/backend/internal/observe/notification/enrich.go new file mode 100644 index 00000000..4f8d3d6a --- /dev/null +++ b/backend/internal/observe/notification/enrich.go @@ -0,0 +1,107 @@ +package notification + +import ( + "fmt" + "strings" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +type intent struct { + Type domain.NotificationType + SessionID domain.SessionID + ProjectID domain.ProjectID + PRURL string + CreatedAt time.Time + + SessionDisplayName string + PRNumber int + PRTitle string + PRSourceBranch string + PRTargetBranch string + Provider string + Repo string +} + +func enrich(in intent) (domain.NotificationRecord, error) { + rec := domain.NotificationRecord{ + SessionID: in.SessionID, + ProjectID: in.ProjectID, + PRURL: strings.TrimSpace(in.PRURL), + Type: in.Type, + Status: domain.NotificationUnread, + CreatedAt: in.CreatedAt, + } + if !in.Type.Valid() { + return domain.NotificationRecord{}, domain.ErrInvalidNotificationType + } + if in.Type != domain.NotificationNeedsInput && rec.PRURL == "" { + return domain.NotificationRecord{}, domain.ErrInvalidNotificationRecord + } + rec.Title = titleForIntent(in) + rec.Body = bodyForIntent(in) + if err := rec.Validate(); err != nil { + return domain.NotificationRecord{}, err + } + return rec, nil +} + +func titleForIntent(in intent) string { + switch in.Type { + case domain.NotificationNeedsInput: + return fmt.Sprintf("%s needs input", sessionLabel(in)) + case domain.NotificationReadyToMerge: + return fmt.Sprintf("%s is ready to merge", prLabel(in)) + case domain.NotificationPRMerged: + return fmt.Sprintf("%s was merged", prLabel(in)) + case domain.NotificationPRClosedUnmerged: + return fmt.Sprintf("%s was closed without merging", prLabel(in)) + default: + return "Notification" + } +} + +func bodyForIntent(in intent) string { + switch in.Type { + case domain.NotificationNeedsInput: + return "The agent is waiting for your response." + case domain.NotificationReadyToMerge: + if s := sessionLabel(in); s != "session" { + return fmt.Sprintf("%s has no known blocking CI or review feedback.", s) + } + return "The pull request has no known blocking CI or review feedback." + case domain.NotificationPRMerged: + if title := strings.TrimSpace(in.PRTitle); title != "" { + return fmt.Sprintf("%s was merged.", title) + } + return "The pull request was merged." + case domain.NotificationPRClosedUnmerged: + if title := strings.TrimSpace(in.PRTitle); title != "" { + return fmt.Sprintf("%s was closed without merging.", title) + } + return "The pull request was closed without merging." + default: + return "" + } +} + +func sessionLabel(in intent) string { + if v := strings.TrimSpace(in.SessionDisplayName); v != "" { + return v + } + if in.SessionID != "" { + return string(in.SessionID) + } + return "session" +} + +func prLabel(in intent) string { + if in.PRNumber > 0 { + return fmt.Sprintf("PR #%d", in.PRNumber) + } + if title := strings.TrimSpace(in.PRTitle); title != "" { + return "PR " + title + } + return "PR" +} diff --git a/backend/internal/observe/notification/projector.go b/backend/internal/observe/notification/projector.go new file mode 100644 index 00000000..a201c6cc --- /dev/null +++ b/backend/internal/observe/notification/projector.go @@ -0,0 +1,253 @@ +// Package notification projects durable CDC facts into unread notification rows. +package notification + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "strings" + "time" + + "github.com/google/uuid" + + "github.com/aoagents/agent-orchestrator/backend/internal/cdc" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +const defaultQueueSize = 1024 + +// Store is the notification projector's storage boundary. +type Store interface { + CreateNotification(ctx context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) + GetSession(ctx context.Context, id domain.SessionID) (domain.SessionRecord, bool, error) + GetPR(ctx context.Context, url string) (domain.PullRequest, bool, error) + ListPRComments(ctx context.Context, prURL string) ([]domain.PullRequestComment, error) +} + +// Subscriber is the live CDC stream consumed by the projector. +type Subscriber interface { + Subscribe(func(cdc.Event)) (unsubscribe func()) +} + +// Projector consumes session/PR CDC facts and writes durable unread notifications. +type Projector struct { + store Store + live Subscriber + logger *slog.Logger + clock func() time.Time + newID func() string + queue int +} + +// Deps configures a Projector. +type Deps struct { + Store Store + Live Subscriber + Logger *slog.Logger + Clock func() time.Time + NewID func() string + Queue int +} + +// New constructs a notification CDC projector. +func New(d Deps) *Projector { + p := &Projector{store: d.Store, live: d.Live, logger: d.Logger, clock: d.Clock, newID: d.NewID, queue: d.Queue} + if p.logger == nil { + p.logger = slog.Default() + } + if p.clock == nil { + p.clock = time.Now + } + if p.newID == nil { + p.newID = func() string { return "ntf_" + uuid.NewString() } + } + if p.queue <= 0 { + p.queue = defaultQueueSize + } + return p +} + +// Start subscribes to live CDC events and processes them until ctx is cancelled. +func (p *Projector) Start(ctx context.Context) <-chan struct{} { + done := make(chan struct{}) + if p == nil || p.live == nil || p.store == nil { + close(done) + return done + } + events := make(chan cdc.Event, p.queue) + unsubscribe := p.live.Subscribe(func(e cdc.Event) { + select { + case events <- e: + default: + p.logger.Warn("notification projector: queue full; dropping CDC event", "seq", e.Seq, "type", e.Type) + } + }) + go func() { + defer close(done) + defer unsubscribe() + for { + select { + case <-ctx.Done(): + return + case e := <-events: + if err := p.Handle(ctx, e); err != nil && !errors.Is(err, context.Canceled) { + p.logger.Warn("notification projector: event failed", "seq", e.Seq, "type", e.Type, "err", err) + } + } + } + }() + return done +} + +// Handle projects a single CDC event. It is exported for deterministic tests. +func (p *Projector) Handle(ctx context.Context, e cdc.Event) error { + if p == nil || p.store == nil { + return errors.New("notification projector: store is required") + } + switch e.Type { + case cdc.EventSessionUpdated: + return p.handleSessionUpdated(ctx, e) + case cdc.EventPRCreated, cdc.EventPRUpdated, cdc.EventPRSessionChanged, cdc.EventPRReviewThreadResolved: + return p.handlePRChanged(ctx, e) + default: + return nil + } +} + +func (p *Projector) handleSessionUpdated(ctx context.Context, e cdc.Event) error { + var payload struct { + Activity string `json:"activity"` + IsTerminated bool `json:"isTerminated"` + } + if err := json.Unmarshal(e.Payload, &payload); err != nil { + return fmt.Errorf("decode session event payload: %w", err) + } + if domain.ActivityState(payload.Activity) != domain.ActivityWaitingInput || payload.IsTerminated || e.SessionID == "" { + return nil + } + rec, ok, err := p.store.GetSession(ctx, domain.SessionID(e.SessionID)) + if err != nil || !ok { + return err + } + return p.create(ctx, intent{ + Type: domain.NotificationNeedsInput, + SessionID: rec.ID, + ProjectID: rec.ProjectID, + CreatedAt: timeOr(e.CreatedAt, p.clock()), + SessionDisplayName: rec.DisplayName, + }) +} + +func (p *Projector) handlePRChanged(ctx context.Context, e cdc.Event) error { + prURL, err := prURLFromEvent(e) + if err != nil || prURL == "" { + return err + } + pr, ok, err := p.store.GetPR(ctx, prURL) + if err != nil || !ok { + return err + } + rec, ok, err := p.store.GetSession(ctx, pr.SessionID) + if err != nil || !ok { + return err + } + comments, err := p.store.ListPRComments(ctx, pr.URL) + if err != nil { + return err + } + in := intentForPR(rec, pr, comments, timeOr(e.CreatedAt, p.clock())) + if in == nil { + return nil + } + return p.create(ctx, *in) +} + +func (p *Projector) create(ctx context.Context, in intent) error { + rec, err := enrich(in) + if err != nil { + return err + } + rec.ID = p.newID() + _, _, err = p.store.CreateNotification(ctx, rec) + return err +} + +func prURLFromEvent(e cdc.Event) (string, error) { + var payload struct { + URL string `json:"url"` + PR string `json:"pr"` + } + if err := json.Unmarshal(e.Payload, &payload); err != nil { + return "", fmt.Errorf("decode pr event payload: %w", err) + } + return firstNonEmpty(payload.URL, payload.PR), nil +} + +func intentForPR(rec domain.SessionRecord, pr domain.PullRequest, comments []domain.PullRequestComment, createdAt time.Time) *intent { + base := intent{ + SessionID: rec.ID, + ProjectID: rec.ProjectID, + PRURL: firstNonEmpty(pr.URL, pr.HTMLURL), + CreatedAt: createdAt, + SessionDisplayName: rec.DisplayName, + PRNumber: pr.Number, + PRTitle: pr.Title, + PRSourceBranch: pr.SourceBranch, + PRTargetBranch: pr.TargetBranch, + Provider: pr.Provider, + Repo: pr.Repo, + } + if pr.Merged { + base.Type = domain.NotificationPRMerged + return &base + } + if pr.Closed { + base.Type = domain.NotificationPRClosedUnmerged + return &base + } + if rec.IsTerminated || rec.Activity.State == domain.ActivityWaitingInput || !prIsReadyToMerge(pr, comments) { + return nil + } + base.Type = domain.NotificationReadyToMerge + return &base +} + +func prIsReadyToMerge(pr domain.PullRequest, comments []domain.PullRequestComment) bool { + if pr.Merged || pr.Closed || pr.Draft { + return false + } + if pr.CI != domain.CIPassing { + return false + } + if pr.Review == domain.ReviewChangesRequest || hasUnresolvedReviewComments(comments) { + return false + } + return pr.Mergeability == domain.MergeMergeable +} + +func hasUnresolvedReviewComments(comments []domain.PullRequestComment) bool { + for _, c := range comments { + if !c.Resolved && !c.IsBot { + return true + } + } + return false +} + +func firstNonEmpty(values ...string) string { + for _, v := range values { + if strings.TrimSpace(v) != "" { + return v + } + } + return "" +} + +func timeOr(t, fallback time.Time) time.Time { + if t.IsZero() { + return fallback + } + return t +} diff --git a/backend/internal/observe/notification/projector_test.go b/backend/internal/observe/notification/projector_test.go new file mode 100644 index 00000000..cbd28124 --- /dev/null +++ b/backend/internal/observe/notification/projector_test.go @@ -0,0 +1,121 @@ +package notification + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/aoagents/agent-orchestrator/backend/internal/cdc" + "github.com/aoagents/agent-orchestrator/backend/internal/domain" +) + +type fakeStore struct { + sessions map[domain.SessionID]domain.SessionRecord + prs map[string]domain.PullRequest + comments map[string][]domain.PullRequestComment + notifications []domain.NotificationRecord +} + +func newFakeStore() *fakeStore { + return &fakeStore{ + sessions: map[domain.SessionID]domain.SessionRecord{}, + prs: map[string]domain.PullRequest{}, + comments: map[string][]domain.PullRequestComment{}, + } +} + +func (f *fakeStore) CreateNotification(_ context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) { + for _, existing := range f.notifications { + if existing.Status == domain.NotificationUnread && existing.SessionID == rec.SessionID && existing.Type == rec.Type && existing.PRURL == rec.PRURL { + return domain.NotificationRecord{}, false, nil + } + } + f.notifications = append(f.notifications, rec) + return rec, true, nil +} + +func (f *fakeStore) GetSession(_ context.Context, id domain.SessionID) (domain.SessionRecord, bool, error) { + rec, ok := f.sessions[id] + return rec, ok, nil +} + +func (f *fakeStore) GetPR(_ context.Context, url string) (domain.PullRequest, bool, error) { + pr, ok := f.prs[url] + return pr, ok, nil +} + +func (f *fakeStore) ListPRComments(_ context.Context, prURL string) ([]domain.PullRequestComment, error) { + return f.comments[prURL], nil +} + +func TestProjectorCreatesNeedsInputFromSessionCDC(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", DisplayName: "checkout-flow"} + now := time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC) + p := New(Deps{Store: st, NewID: func() string { return "ntf_1" }}) + + if err := p.Handle(context.Background(), event(t, 1, cdc.EventSessionUpdated, "mer", "mer-1", now, map[string]any{ + "id": "mer-1", "activity": string(domain.ActivityWaitingInput), "isTerminated": false, + })); err != nil { + t.Fatalf("Handle: %v", err) + } + if len(st.notifications) != 1 { + t.Fatalf("notifications = %+v", st.notifications) + } + got := st.notifications[0] + if got.Type != domain.NotificationNeedsInput || got.Title != "checkout-flow needs input" || got.CreatedAt != now { + t.Fatalf("notification = %+v", got) + } +} + +func TestProjectorCreatesPRNotificationsFromPRCDC(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", DisplayName: "checkout-flow"} + st.prs["pr1"] = domain.PullRequest{URL: "pr1", SessionID: "mer-1", Number: 42, Title: "checkout", CI: domain.CIPassing, Review: domain.ReviewApproved, Mergeability: domain.MergeMergeable} + p := New(Deps{Store: st, NewID: func() string { return "ntf_1" }}) + + if err := p.Handle(context.Background(), event(t, 1, cdc.EventPRUpdated, "mer", "mer-1", time.Now(), map[string]any{"url": "pr1"})); err != nil { + t.Fatalf("Handle: %v", err) + } + if len(st.notifications) != 1 || st.notifications[0].Type != domain.NotificationReadyToMerge || st.notifications[0].Title != "PR #42 is ready to merge" { + t.Fatalf("notifications = %+v", st.notifications) + } +} + +func TestProjectorDoesNotCreateReadyNotificationWhenCIIsPending(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer"} + st.prs["pr1"] = domain.PullRequest{URL: "pr1", SessionID: "mer-1", Number: 42, CI: domain.CIPending, Review: domain.ReviewApproved, Mergeability: domain.MergeMergeable} + p := New(Deps{Store: st}) + + if err := p.Handle(context.Background(), event(t, 1, cdc.EventPRUpdated, "mer", "mer-1", time.Now(), map[string]any{"url": "pr1"})); err != nil { + t.Fatalf("Handle: %v", err) + } + if len(st.notifications) != 0 { + t.Fatalf("notifications = %+v", st.notifications) + } +} + +func TestProjectorCreatesMergedNotificationForTerminatedSession(t *testing.T) { + st := newFakeStore() + st.sessions["mer-1"] = domain.SessionRecord{ID: "mer-1", ProjectID: "mer", IsTerminated: true} + st.prs["pr1"] = domain.PullRequest{URL: "pr1", SessionID: "mer-1", Number: 42, Merged: true} + p := New(Deps{Store: st, NewID: func() string { return "ntf_1" }}) + + if err := p.Handle(context.Background(), event(t, 1, cdc.EventPRUpdated, "mer", "mer-1", time.Now(), map[string]any{"url": "pr1"})); err != nil { + t.Fatalf("Handle: %v", err) + } + if len(st.notifications) != 1 || st.notifications[0].Type != domain.NotificationPRMerged { + t.Fatalf("notifications = %+v", st.notifications) + } +} + +func event(t *testing.T, seq int64, typ cdc.EventType, projectID, sessionID string, at time.Time, payload map[string]any) cdc.Event { + t.Helper() + raw, err := json.Marshal(payload) + if err != nil { + t.Fatal(err) + } + return cdc.Event{Seq: seq, ProjectID: projectID, SessionID: sessionID, Type: typ, Payload: raw, CreatedAt: at} +} diff --git a/backend/internal/ports/notifications.go b/backend/internal/ports/notifications.go deleted file mode 100644 index 7f7c153e..00000000 --- a/backend/internal/ports/notifications.go +++ /dev/null @@ -1,27 +0,0 @@ -package ports - -import ( - "time" - - "github.com/aoagents/agent-orchestrator/backend/internal/domain" -) - -// NotificationIntent is the lifecycle-to-notification-service contract. It is -// not an HTTP DTO; lifecycle fills it from facts it already has after the -// underlying session/PR state write succeeds. -type NotificationIntent struct { - Type domain.NotificationType - SessionID domain.SessionID - ProjectID domain.ProjectID - PRURL string - CreatedAt time.Time - - // Enrichment hints. These avoid storage reads on the hot path. - SessionDisplayName string - PRNumber int - PRTitle string - PRSourceBranch string - PRTargetBranch string - Provider string - Repo string -} diff --git a/backend/internal/service/notification/enrich.go b/backend/internal/service/notification/enrich.go deleted file mode 100644 index dc6729c0..00000000 --- a/backend/internal/service/notification/enrich.go +++ /dev/null @@ -1,90 +0,0 @@ -package notification - -import ( - "fmt" - "strings" - - "github.com/aoagents/agent-orchestrator/backend/internal/domain" -) - -func enrich(intent Intent) (domain.NotificationRecord, error) { - rec := domain.NotificationRecord{ - SessionID: intent.SessionID, - ProjectID: intent.ProjectID, - PRURL: strings.TrimSpace(intent.PRURL), - Type: intent.Type, - Status: domain.NotificationUnread, - CreatedAt: intent.CreatedAt, - } - if !intent.Type.Valid() { - return domain.NotificationRecord{}, domain.ErrInvalidNotificationType - } - if intent.Type != domain.NotificationNeedsInput && rec.PRURL == "" { - return domain.NotificationRecord{}, domain.ErrInvalidNotificationRecord - } - rec.Title = titleForIntent(intent) - rec.Body = bodyForIntent(intent) - if err := rec.Validate(); err != nil { - return domain.NotificationRecord{}, err - } - return rec, nil -} - -func titleForIntent(intent Intent) string { - switch intent.Type { - case domain.NotificationNeedsInput: - return fmt.Sprintf("%s needs input", sessionLabel(intent)) - case domain.NotificationReadyToMerge: - return fmt.Sprintf("%s is ready to merge", prLabel(intent)) - case domain.NotificationPRMerged: - return fmt.Sprintf("%s was merged", prLabel(intent)) - case domain.NotificationPRClosedUnmerged: - return fmt.Sprintf("%s was closed without merging", prLabel(intent)) - default: - return "Notification" - } -} - -func bodyForIntent(intent Intent) string { - switch intent.Type { - case domain.NotificationNeedsInput: - return "The agent is waiting for your response." - case domain.NotificationReadyToMerge: - if s := sessionLabel(intent); s != "session" { - return fmt.Sprintf("%s has no known blocking CI or review feedback.", s) - } - return "The pull request has no known blocking CI or review feedback." - case domain.NotificationPRMerged: - if title := strings.TrimSpace(intent.PRTitle); title != "" { - return fmt.Sprintf("%s was merged.", title) - } - return "The pull request was merged." - case domain.NotificationPRClosedUnmerged: - if title := strings.TrimSpace(intent.PRTitle); title != "" { - return fmt.Sprintf("%s was closed without merging.", title) - } - return "The pull request was closed without merging." - default: - return "" - } -} - -func sessionLabel(intent Intent) string { - if v := strings.TrimSpace(intent.SessionDisplayName); v != "" { - return v - } - if intent.SessionID != "" { - return string(intent.SessionID) - } - return "session" -} - -func prLabel(intent Intent) string { - if intent.PRNumber > 0 { - return fmt.Sprintf("PR #%d", intent.PRNumber) - } - if title := strings.TrimSpace(intent.PRTitle); title != "" { - return "PR " + title - } - return "PR" -} diff --git a/backend/internal/service/notification/service.go b/backend/internal/service/notification/service.go index 29ab980a..25f92edc 100644 --- a/backend/internal/service/notification/service.go +++ b/backend/internal/service/notification/service.go @@ -3,10 +3,6 @@ package notification import ( "context" "errors" - "fmt" - "time" - - "github.com/google/uuid" "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) @@ -18,54 +14,19 @@ const ( MaxListLimit = 100 ) -// Manager validates lifecycle intents, enriches them into stored rows, and persists -// unread notifications. +// Manager reads stored notifications for REST controllers. type Manager struct { store Store - clock func() time.Time - newID func() string } // Deps configures a Manager. type Deps struct { Store Store - Clock func() time.Time - NewID func() string } -// New constructs a Manager with production defaults for optional collaborators. +// New constructs a read-only notification Manager. func New(d Deps) *Manager { - m := &Manager{store: d.Store, clock: d.Clock, newID: d.NewID} - if m.clock == nil { - m.clock = time.Now - } - if m.newID == nil { - m.newID = func() string { return "ntf_" + uuid.NewString() } - } - return m -} - -// Notify stores one notification intent. Duplicate unread rows are treated as a clean no-op. -func (m *Manager) Notify(ctx context.Context, intent Intent) error { - if m == nil || m.store == nil { - return errors.New("notification: store is required") - } - if intent.CreatedAt.IsZero() { - intent.CreatedAt = m.clock().UTC() - } - rec, err := enrich(intent) - if err != nil { - return fmt.Errorf("notification enrich: %w", err) - } - rec.ID = m.newID() - _, inserted, err := m.store.CreateNotification(ctx, rec) - if err != nil { - return fmt.Errorf("notification store: %w", err) - } - if !inserted { - return nil - } - return nil + return &Manager{store: d.Store} } // ListUnread returns unread notifications newest-first. diff --git a/backend/internal/service/notification/service_test.go b/backend/internal/service/notification/service_test.go index 1b2fc52b..3330fa53 100644 --- a/backend/internal/service/notification/service_test.go +++ b/backend/internal/service/notification/service_test.go @@ -2,7 +2,6 @@ package notification import ( "context" - "errors" "testing" "time" @@ -10,66 +9,16 @@ import ( ) type fakeStore struct { - rows []domain.NotificationRecord - duplicate bool - err error + rows []domain.NotificationRecord + err error } -func (f *fakeStore) CreateNotification(_ context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) { - if f.err != nil { - return domain.NotificationRecord{}, false, f.err - } - if f.duplicate { - return domain.NotificationRecord{}, false, nil - } - f.rows = append(f.rows, rec) - return rec, true, nil +func (f *fakeStore) CreateNotification(context.Context, domain.NotificationRecord) (domain.NotificationRecord, bool, error) { + return domain.NotificationRecord{}, false, nil } func (f *fakeStore) ListUnreadNotifications(_ context.Context, _ int) ([]domain.NotificationRecord, error) { - return f.rows, nil -} - -func TestManagerNotifyPersistsNotification(t *testing.T) { - st := &fakeStore{} - now := time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC) - mgr := New(Deps{Store: st, Clock: func() time.Time { return now }, NewID: func() string { return "ntf_1" }}) - - if err := mgr.Notify(context.Background(), Intent{ - Type: domain.NotificationNeedsInput, - SessionID: "mer-1", - ProjectID: "mer", - SessionDisplayName: "checkout-flow", - }); err != nil { - t.Fatalf("Notify: %v", err) - } - if len(st.rows) != 1 { - t.Fatalf("stored rows = %d, want 1", len(st.rows)) - } - if got := st.rows[0]; got.ID != "ntf_1" || got.CreatedAt != now || got.Status != domain.NotificationUnread || got.Title != "checkout-flow needs input" { - t.Fatalf("stored notification = %+v", got) - } -} - -func TestManagerNotifyDuplicateIsIgnored(t *testing.T) { - st := &fakeStore{duplicate: true} - mgr := New(Deps{Store: st, Clock: func() time.Time { return time.Now() }, NewID: func() string { return "ntf_1" }}) - - err := mgr.Notify(context.Background(), Intent{Type: domain.NotificationNeedsInput, SessionID: "mer-1", ProjectID: "mer", CreatedAt: time.Now()}) - if err != nil { - t.Fatalf("Notify duplicate: %v", err) - } - if len(st.rows) != 0 { - t.Fatalf("duplicate should not persist, got %+v", st.rows) - } -} - -func TestManagerNotifyRejectsUnknownType(t *testing.T) { - mgr := New(Deps{Store: &fakeStore{}, Clock: func() time.Time { return time.Now() }}) - err := mgr.Notify(context.Background(), Intent{Type: "surprise", SessionID: "mer-1", ProjectID: "mer"}) - if !errors.Is(err, domain.ErrInvalidNotificationType) { - t.Fatalf("err = %v, want invalid type", err) - } + return f.rows, f.err } func TestListUnreadAddsTargets(t *testing.T) { @@ -86,3 +35,10 @@ func TestListUnreadAddsTargets(t *testing.T) { t.Fatalf("targets = %+v", got) } } + +func TestListUnreadRequiresStore(t *testing.T) { + _, err := New(Deps{}).ListUnread(context.Background(), ListFilter{}) + if err == nil { + t.Fatal("want missing store error") + } +} diff --git a/backend/internal/service/notification/store.go b/backend/internal/service/notification/store.go index 022def81..58be2625 100644 --- a/backend/internal/service/notification/store.go +++ b/backend/internal/service/notification/store.go @@ -6,8 +6,7 @@ import ( "github.com/aoagents/agent-orchestrator/backend/internal/domain" ) -// Store is the notification service's persistence surface. +// Store is the notification service's read persistence surface. type Store interface { - CreateNotification(ctx context.Context, rec domain.NotificationRecord) (domain.NotificationRecord, bool, error) ListUnreadNotifications(ctx context.Context, limit int) ([]domain.NotificationRecord, error) } diff --git a/backend/internal/service/notification/types.go b/backend/internal/service/notification/types.go index c1c6d4c2..9d2673d0 100644 --- a/backend/internal/service/notification/types.go +++ b/backend/internal/service/notification/types.go @@ -1,15 +1,7 @@ -// Package notification enriches lifecycle notification intents, persists unread -// rows, and dispatches created notifications to dashboard-facing publishers. +// Package notification exposes read-only notification DTOs for REST controllers. package notification -import ( - "github.com/aoagents/agent-orchestrator/backend/internal/domain" - "github.com/aoagents/agent-orchestrator/backend/internal/ports" -) - -// Intent is the service boundary shape. It aliases the lifecycle contract so -// Manager.Notify also satisfies lifecycle's NotificationSink without copying. -type Intent = ports.NotificationIntent +import "github.com/aoagents/agent-orchestrator/backend/internal/domain" // TargetKind describes what a dashboard should navigate to for a notification. type TargetKind string diff --git a/backend/internal/storage/sqlite/migrations/0011_notifications.sql b/backend/internal/storage/sqlite/migrations/0011_notifications.sql index cd12d7d0..90c2dacb 100644 --- a/backend/internal/storage/sqlite/migrations/0011_notifications.sql +++ b/backend/internal/storage/sqlite/migrations/0011_notifications.sql @@ -1,4 +1,187 @@ -- +goose Up +-- +goose StatementBegin +DROP TRIGGER IF EXISTS pr_review_threads_cdc_update; +DROP TRIGGER IF EXISTS pr_review_threads_cdc_insert; +DROP TRIGGER IF EXISTS sessions_cdc_insert; +DROP TRIGGER IF EXISTS sessions_cdc_update; +DROP TRIGGER IF EXISTS pr_cdc_insert; +DROP TRIGGER IF EXISTS pr_cdc_update; +DROP TRIGGER IF EXISTS pr_session_cdc_update; +DROP TRIGGER IF EXISTS pr_checks_cdc_insert; +DROP TRIGGER IF EXISTS pr_checks_cdc_update; + +CREATE TABLE change_log_new ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, + project_id TEXT NOT NULL REFERENCES projects (id), + session_id TEXT REFERENCES sessions (id), + event_type TEXT NOT NULL + CHECK (event_type IN ( + 'session_created', + 'session_updated', + 'pr_created', + 'pr_updated', + 'pr_check_recorded', + 'pr_session_changed', + 'pr_review_thread_added', + 'pr_review_thread_resolved', + 'notification_created' + )), + payload TEXT NOT NULL CHECK (json_valid(payload)), + created_at TIMESTAMP NOT NULL DEFAULT (datetime('now')) +); + +INSERT INTO change_log_new (seq, project_id, session_id, event_type, payload, created_at) +SELECT seq, project_id, session_id, event_type, payload, created_at +FROM change_log; + +DROP INDEX IF EXISTS idx_change_log_project; +DROP TABLE change_log; +ALTER TABLE change_log_new RENAME TO change_log; +CREATE INDEX idx_change_log_project ON change_log (project_id, seq); +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_review_threads_cdc_insert +AFTER INSERT ON pr_review_threads +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_review_thread_added', + json_object( + 'pr', NEW.pr_url, + 'thread', NEW.thread_id, + 'path', NEW.path, + 'line', NEW.line, + 'resolved', json(CASE WHEN NEW.resolved THEN 'true' ELSE 'false' END), + 'isBot', json(CASE WHEN NEW.is_bot THEN 'true' ELSE 'false' END) + ), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_review_threads_cdc_update +AFTER UPDATE ON pr_review_threads +WHEN OLD.resolved <> NEW.resolved +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_review_thread_resolved', + json_object( + 'pr', NEW.pr_url, + 'thread', NEW.thread_id, + 'path', NEW.path, + 'line', NEW.line, + 'resolved', json(CASE WHEN NEW.resolved THEN 'true' ELSE 'false' END) + ), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER sessions_cdc_insert +AFTER INSERT ON sessions +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES (NEW.project_id, NEW.id, 'session_created', + json_object('id', NEW.id, 'activity', NEW.activity_state, 'isTerminated', json(CASE WHEN NEW.is_terminated THEN 'true' ELSE 'false' END)), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER sessions_cdc_update +AFTER UPDATE ON sessions +WHEN OLD.activity_state <> NEW.activity_state + OR OLD.is_terminated <> NEW.is_terminated + OR (OLD.first_signal_at IS NULL AND NEW.first_signal_at IS NOT NULL) +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES (NEW.project_id, NEW.id, 'session_updated', + json_object('id', NEW.id, 'activity', NEW.activity_state, 'isTerminated', json(CASE WHEN NEW.is_terminated THEN 'true' ELSE 'false' END)), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_cdc_insert +AFTER INSERT ON pr +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ((SELECT project_id FROM sessions WHERE id = NEW.session_id), NEW.session_id, 'pr_created', + json_object('url', NEW.url, 'session', NEW.session_id, 'state', NEW.pr_state, + 'ci', NEW.ci_state, 'review', NEW.review_decision, 'mergeability', NEW.mergeability), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_cdc_update +AFTER UPDATE ON pr +WHEN OLD.pr_state <> NEW.pr_state + OR OLD.ci_state <> NEW.ci_state + OR OLD.review_decision <> NEW.review_decision + OR OLD.mergeability <> NEW.mergeability +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ((SELECT project_id FROM sessions WHERE id = NEW.session_id), NEW.session_id, 'pr_updated', + json_object('url', NEW.url, 'session', NEW.session_id, 'state', NEW.pr_state, + 'ci', NEW.ci_state, 'review', NEW.review_decision, 'mergeability', NEW.mergeability), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_session_cdc_update +AFTER UPDATE ON pr +WHEN OLD.session_id <> NEW.session_id +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT project_id FROM sessions WHERE id = NEW.session_id), + NEW.session_id, + 'pr_session_changed', + json_object( + 'url', NEW.url, + 'fromSession', OLD.session_id, + 'toSession', NEW.session_id), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_checks_cdc_insert +AFTER INSERT ON pr_checks +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_check_recorded', + json_object('pr', NEW.pr_url, 'name', NEW.name, 'commit', NEW.commit_hash, 'status', NEW.status), + NEW.created_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_checks_cdc_update +AFTER UPDATE ON pr_checks +WHEN OLD.status <> NEW.status +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_check_recorded', + json_object('pr', NEW.pr_url, 'name', NEW.name, 'commit', NEW.commit_hash, 'status', NEW.status), + datetime('now')); +END; +-- +goose StatementEnd + -- +goose StatementBegin CREATE TABLE notifications ( id TEXT PRIMARY KEY, @@ -15,7 +198,7 @@ CREATE TABLE notifications ( ), title TEXT NOT NULL, body TEXT NOT NULL DEFAULT '', - status TEXT NOT NULL DEFAULT 'unread', + status TEXT NOT NULL DEFAULT 'unread' CHECK (status IN ('read', 'unread')), created_at TIMESTAMP NOT NULL ); @@ -27,9 +210,202 @@ CREATE UNIQUE INDEX idx_notifications_unread_dedupe WHERE status = 'unread'; -- +goose StatementEnd +-- +goose StatementBegin +CREATE TRIGGER notifications_cdc_insert +AFTER INSERT ON notifications +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES (NEW.project_id, NEW.session_id, 'notification_created', + json_object('id', NEW.id, 'session', NEW.session_id, 'type', NEW.type, 'status', NEW.status), + NEW.created_at); +END; +-- +goose StatementEnd + -- +goose Down -- +goose StatementBegin +DROP TRIGGER IF EXISTS notifications_cdc_insert; DROP INDEX IF EXISTS idx_notifications_unread_dedupe; DROP INDEX IF EXISTS idx_notifications_status; DROP TABLE IF EXISTS notifications; + +DROP TRIGGER IF EXISTS pr_review_threads_cdc_update; +DROP TRIGGER IF EXISTS pr_review_threads_cdc_insert; +DROP TRIGGER IF EXISTS sessions_cdc_insert; +DROP TRIGGER IF EXISTS sessions_cdc_update; +DROP TRIGGER IF EXISTS pr_cdc_insert; +DROP TRIGGER IF EXISTS pr_cdc_update; +DROP TRIGGER IF EXISTS pr_session_cdc_update; +DROP TRIGGER IF EXISTS pr_checks_cdc_insert; +DROP TRIGGER IF EXISTS pr_checks_cdc_update; + +CREATE TABLE change_log_old ( + seq INTEGER PRIMARY KEY AUTOINCREMENT, + project_id TEXT NOT NULL REFERENCES projects (id), + session_id TEXT REFERENCES sessions (id), + event_type TEXT NOT NULL + CHECK (event_type IN ( + 'session_created', + 'session_updated', + 'pr_created', + 'pr_updated', + 'pr_check_recorded', + 'pr_session_changed', + 'pr_review_thread_added', + 'pr_review_thread_resolved' + )), + payload TEXT NOT NULL CHECK (json_valid(payload)), + created_at TIMESTAMP NOT NULL DEFAULT (datetime('now')) +); + +INSERT INTO change_log_old (seq, project_id, session_id, event_type, payload, created_at) +SELECT seq, project_id, session_id, event_type, payload, created_at +FROM change_log +WHERE event_type <> 'notification_created'; + +DROP INDEX IF EXISTS idx_change_log_project; +DROP TABLE change_log; +ALTER TABLE change_log_old RENAME TO change_log; +CREATE INDEX idx_change_log_project ON change_log (project_id, seq); +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_review_threads_cdc_insert +AFTER INSERT ON pr_review_threads +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_review_thread_added', + json_object( + 'pr', NEW.pr_url, + 'thread', NEW.thread_id, + 'path', NEW.path, + 'line', NEW.line, + 'resolved', json(CASE WHEN NEW.resolved THEN 'true' ELSE 'false' END), + 'isBot', json(CASE WHEN NEW.is_bot THEN 'true' ELSE 'false' END) + ), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_review_threads_cdc_update +AFTER UPDATE ON pr_review_threads +WHEN OLD.resolved <> NEW.resolved +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_review_thread_resolved', + json_object( + 'pr', NEW.pr_url, + 'thread', NEW.thread_id, + 'path', NEW.path, + 'line', NEW.line, + 'resolved', json(CASE WHEN NEW.resolved THEN 'true' ELSE 'false' END) + ), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER sessions_cdc_insert +AFTER INSERT ON sessions +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES (NEW.project_id, NEW.id, 'session_created', + json_object('id', NEW.id, 'activity', NEW.activity_state, 'isTerminated', json(CASE WHEN NEW.is_terminated THEN 'true' ELSE 'false' END)), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER sessions_cdc_update +AFTER UPDATE ON sessions +WHEN OLD.activity_state <> NEW.activity_state + OR OLD.is_terminated <> NEW.is_terminated + OR (OLD.first_signal_at IS NULL AND NEW.first_signal_at IS NOT NULL) +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES (NEW.project_id, NEW.id, 'session_updated', + json_object('id', NEW.id, 'activity', NEW.activity_state, 'isTerminated', json(CASE WHEN NEW.is_terminated THEN 'true' ELSE 'false' END)), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_cdc_insert +AFTER INSERT ON pr +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ((SELECT project_id FROM sessions WHERE id = NEW.session_id), NEW.session_id, 'pr_created', + json_object('url', NEW.url, 'session', NEW.session_id, 'state', NEW.pr_state, + 'ci', NEW.ci_state, 'review', NEW.review_decision, 'mergeability', NEW.mergeability), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_cdc_update +AFTER UPDATE ON pr +WHEN OLD.pr_state <> NEW.pr_state + OR OLD.ci_state <> NEW.ci_state + OR OLD.review_decision <> NEW.review_decision + OR OLD.mergeability <> NEW.mergeability +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ((SELECT project_id FROM sessions WHERE id = NEW.session_id), NEW.session_id, 'pr_updated', + json_object('url', NEW.url, 'session', NEW.session_id, 'state', NEW.pr_state, + 'ci', NEW.ci_state, 'review', NEW.review_decision, 'mergeability', NEW.mergeability), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_session_cdc_update +AFTER UPDATE ON pr +WHEN OLD.session_id <> NEW.session_id +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT project_id FROM sessions WHERE id = NEW.session_id), + NEW.session_id, + 'pr_session_changed', + json_object( + 'url', NEW.url, + 'fromSession', OLD.session_id, + 'toSession', NEW.session_id), + NEW.updated_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_checks_cdc_insert +AFTER INSERT ON pr_checks +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_check_recorded', + json_object('pr', NEW.pr_url, 'name', NEW.name, 'commit', NEW.commit_hash, 'status', NEW.status), + NEW.created_at); +END; +-- +goose StatementEnd + +-- +goose StatementBegin +CREATE TRIGGER pr_checks_cdc_update +AFTER UPDATE ON pr_checks +WHEN OLD.status <> NEW.status +BEGIN + INSERT INTO change_log (project_id, session_id, event_type, payload, created_at) + VALUES ( + (SELECT s.project_id FROM pr p JOIN sessions s ON s.id = p.session_id WHERE p.url = NEW.pr_url), + (SELECT session_id FROM pr WHERE url = NEW.pr_url), + 'pr_check_recorded', + json_object('pr', NEW.pr_url, 'name', NEW.name, 'commit', NEW.commit_hash, 'status', NEW.status), + datetime('now')); +END; -- +goose StatementEnd diff --git a/backend/internal/storage/sqlite/store/notification_store_test.go b/backend/internal/storage/sqlite/store/notification_store_test.go index 0b2ebf9b..3e00c077 100644 --- a/backend/internal/storage/sqlite/store/notification_store_test.go +++ b/backend/internal/storage/sqlite/store/notification_store_test.go @@ -2,6 +2,7 @@ package store_test import ( "context" + "encoding/json" "errors" "testing" "time" @@ -49,6 +50,46 @@ func TestNotificationStore_InsertListAndDedupe(t *testing.T) { } } +func TestNotificationStore_InsertEmitsCDC(t *testing.T) { + s := newTestStore(t) + ctx := context.Background() + seedProject(t, s, "mer") + sess, err := s.CreateSession(ctx, sampleRecord("mer")) + if err != nil { + t.Fatalf("create session: %v", err) + } + rec := domain.NotificationRecord{ + ID: "ntf_1", + SessionID: sess.ID, + ProjectID: sess.ProjectID, + Type: domain.NotificationNeedsInput, + Title: "checkout-flow needs input", + Status: domain.NotificationUnread, + CreatedAt: time.Now().UTC().Truncate(time.Second), + } + if _, inserted, err := s.CreateNotification(ctx, rec); err != nil || !inserted { + t.Fatalf("CreateNotification inserted=%v err=%v", inserted, err) + } + events, err := s.EventsAfter(ctx, 0, 100) + if err != nil { + t.Fatalf("EventsAfter: %v", err) + } + for _, ev := range events { + if ev.Type != "notification_created" { + continue + } + var payload map[string]string + if err := json.Unmarshal(ev.Payload, &payload); err != nil { + t.Fatalf("payload JSON: %v", err) + } + if ev.ProjectID != string(sess.ProjectID) || ev.SessionID != string(sess.ID) || payload["id"] != rec.ID { + t.Fatalf("notification event = %+v payload=%+v", ev, payload) + } + return + } + t.Fatalf("notification_created event not found: %+v", events) +} + func TestNotificationStore_ListUnreadNewestFirstAcrossProjects(t *testing.T) { s := newTestStore(t) ctx := context.Background()