Allow Rootly plugin to collect incidents without service identifiers assigned (#8892)

* fix: map Rootly 'completed' status to DONE in incident converter

The mapStatus function was missing 'completed' as a terminal status,
causing incidents with that status to be written as IN_PROGRESS instead
of DONE.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: use updated_date as MTTR fallback for Rootly 'completed' incidents

Rootly's 'completed' status represents a fully resolved incident that has
gone through post-incident review. These incidents may not have a
resolved_date populated. Fall back to updated_date so MTTR is computed
correctly instead of showing the incident as unresolved.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* fix: remove service filter requirement from Rootly incident collection

Allow collecting all incidents without filtering by service ID.
Previously, all incidents required a service association to be collected;
incidents created without service tags (common in Rootly) were silently
dropped.

Changes:
- ValidateTaskOptions: service_id is now optional (no longer required)
- GetParams: falls back to scope_id 'all' when service_id is empty
- buildIncidentsQuery: only adds filter[service_ids] when service_id is set
- extractRootlyIncident: skips the service-membership guard when
  collecting globally (service_id = '') to avoid false drops

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: brianfeucht

backend/plugins/rootly/tasks/incidents_collector.go

@@ -126,7 +126,9 @@ func CollectIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 // `filter[service_ids]`) is caught by a unit test.
 func buildIncidentsQuery(serviceId string, pageSize, pageNumber int, createdAfter *time.Time) url.Values {
 	query := url.Values{}
-	query.Set("filter[service_ids]", serviceId)
+	if serviceId != "" {
+		query.Set("filter[service_ids]", serviceId)
+	}
 	query.Set("page[size]", fmt.Sprintf("%d", pageSize))
 	// Rootly's JSON:API pagination is 1-based.
 	query.Set("page[number]", fmt.Sprintf("%d", pageNumber))

backend/plugins/rootly/tasks/incidents_collector_test.go

@@ -34,6 +34,13 @@ func TestBuildIncidentsQuery_FirstPageNoSince(t *testing.T) {
 	assert.Equal(t, "", q.Get("filter[services]"), "regression guard: must be filter[service_ids], not filter[services]")
 }
 
+func TestBuildIncidentsQuery_NoServiceFilter(t *testing.T) {
+	q := buildIncidentsQuery("", 100, 1, nil)
+	assert.Equal(t, "", q.Get("filter[service_ids]"), "empty serviceId must omit the service filter entirely")
+	assert.Equal(t, "100", q.Get("page[size]"))
+	assert.Equal(t, "1", q.Get("page[number]"))
+}
+
 func TestBuildIncidentsQuery_SubsequentPage(t *testing.T) {
 	q := buildIncidentsQuery("svc_42", 100, 3, nil)
 	assert.Equal(t, "3", q.Get("page[number]"))

backend/plugins/rootly/tasks/incidents_converter.go

@@ -90,7 +90,7 @@ func ConvertIncidents(taskCtx plugin.SubTaskContext) errors.Error {
 				logger.Warn(nil, "unknown rootly incident status: %s", incident.Status)
 			}
 
-			leadTime, resolutionDate := computeLeadTime(incident.StartedDate, incident.ResolvedDate)
+			leadTime, resolutionDate := computeLeadTime(incident.StartedDate, incident.ResolvedDate, incident.UpdatedDate, incident.Status)
 
 			domainIssueId := idGen.Generate(data.Options.ConnectionId, incident.Id)
 
@@ -161,7 +161,7 @@ func mapStatus(status string) (mapped string, known bool) {
 		return ticket.TODO, true
 	case "mitigated":
 		return ticket.IN_PROGRESS, true
-	case "resolved", "closed", "cancelled":
+	case "resolved", "closed", "cancelled", "completed":
 		return ticket.DONE, true
 	default:
 		return ticket.IN_PROGRESS, false
@@ -183,18 +183,24 @@ func mapSeverityToPriority(severity string) string {
 	}
 }
 
-func computeLeadTime(started time.Time, resolved *time.Time) (*uint, *time.Time) {
-	if resolved == nil {
+func computeLeadTime(started time.Time, resolved *time.Time, updated time.Time, status string) (*uint, *time.Time) {
+	// For "completed" incidents Rootly may not populate resolved_date; fall
+	// back to updated_date which reflects when the incident was last actioned.
+	effective := resolved
+	if effective == nil && status == "completed" {
+		effective = &updated
+	}
+	if effective == nil {
 		return nil, nil
 	}
 	// Clock skew / backfill can place resolved before started. A naive
 	// uint() cast on a negative duration wraps to huge garbage and
 	// silently corrupts MTTR; treat as unresolved instead.
-	if resolved.Before(started) {
+	if effective.Before(started) {
 		return nil, nil
 	}
-	minutes := uint(resolved.Sub(started).Minutes())
-	resolutionDate := *resolved
+	minutes := uint(effective.Sub(started).Minutes())
+	resolutionDate := *effective
 	return &minutes, &resolutionDate
 }
 

backend/plugins/rootly/tasks/incidents_converter_test.go

@@ -85,7 +85,8 @@ func TestMapSeverityToPriority(t *testing.T) {
 func TestComputeLeadTime_Resolved(t *testing.T) {
 	started := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
 	resolved := time.Date(2026, 5, 10, 11, 30, 0, 0, time.UTC)
-	leadTime, resolutionDate := computeLeadTime(started, &resolved)
+	updated := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC)
+	leadTime, resolutionDate := computeLeadTime(started, &resolved, updated, "resolved")
 	require.NotNil(t, leadTime)
 	require.NotNil(t, resolutionDate)
 	assert.Equal(t, uint(90), *leadTime)
@@ -94,15 +95,26 @@ func TestComputeLeadTime_Resolved(t *testing.T) {
 
 func TestComputeLeadTime_Unresolved(t *testing.T) {
 	started := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
-	leadTime, resolutionDate := computeLeadTime(started, nil)
+	updated := time.Date(2026, 5, 10, 12, 0, 0, 0, time.UTC)
+	leadTime, resolutionDate := computeLeadTime(started, nil, updated, "started")
 	assert.Nil(t, leadTime)
 	assert.Nil(t, resolutionDate)
 }
 
+func TestComputeLeadTime_CompletedFallsBackToUpdated(t *testing.T) {
+	started := time.Date(2026, 3, 27, 20, 0, 0, 0, time.UTC)
+	updated := time.Date(2026, 3, 31, 14, 0, 0, 0, time.UTC)
+	leadTime, resolutionDate := computeLeadTime(started, nil, updated, "completed")
+	require.NotNil(t, leadTime)
+	require.NotNil(t, resolutionDate)
+	assert.Equal(t, updated, *resolutionDate)
+}
+
 func TestComputeLeadTime_ZeroDuration(t *testing.T) {
 	started := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
 	resolved := started
-	leadTime, resolutionDate := computeLeadTime(started, &resolved)
+	updated := started
+	leadTime, resolutionDate := computeLeadTime(started, &resolved, updated, "resolved")
 	require.NotNil(t, leadTime)
 	require.NotNil(t, resolutionDate)
 	assert.Equal(t, uint(0), *leadTime)
@@ -111,7 +123,8 @@ func TestComputeLeadTime_ZeroDuration(t *testing.T) {
 func TestComputeLeadTime_ResolvedBeforeStarted(t *testing.T) {
 	started := time.Date(2026, 5, 10, 11, 0, 0, 0, time.UTC)
 	resolved := time.Date(2026, 5, 10, 10, 0, 0, 0, time.UTC)
-	leadTime, resolutionDate := computeLeadTime(started, &resolved)
+	updated := started
+	leadTime, resolutionDate := computeLeadTime(started, &resolved, updated, "resolved")
 	assert.Nil(t, leadTime)
 	assert.Nil(t, resolutionDate)
 }

backend/plugins/rootly/tasks/incidents_extractor.go

@@ -64,9 +64,12 @@ func extractRootlyIncident(rawData []byte, op *RootlyOptions) ([]interface{}, er
 
 	// Safety net: filter[service_ids] in the collector is the primary
 	// scope filter, but a regression there would let multi-service
-	// incidents leak into a wrong scope's tool table.
-	if services := rawIncident.Relationships.Services.Data; len(services) > 0 && !containsServiceId(services, op.ServiceId) {
-		return nil, nil
+	// incidents leak into a wrong scope's tool table. When ServiceId is
+	// empty we are collecting all incidents globally, so skip this check.
+	if op.ServiceId != "" {
+		if services := rawIncident.Relationships.Services.Data; len(services) > 0 && !containsServiceId(services, op.ServiceId) {
+			return nil, nil
+		}
 	}
 
 	if rawIncident.Attributes.StartedAt.IsZero() {

backend/plugins/rootly/tasks/task_data.go

@@ -37,9 +37,13 @@ type RootlyTaskData struct {
 }
 
 func (p *RootlyOptions) GetParams() any {
+	scopeId := p.ServiceId
+	if scopeId == "" {
+		scopeId = "all"
+	}
 	return models.RootlyParams{
 		ConnectionId: p.ConnectionId,
-		ScopeId:      p.ServiceId,
+		ScopeId:      scopeId,
 	}
 }
 
@@ -65,9 +69,6 @@ func DecodeTaskOptions(options map[string]interface{}) (*RootlyOptions, errors.E
 }
 
 func ValidateTaskOptions(op *RootlyOptions) errors.Error {
-	if op.ServiceId == "" {
-		return errors.BadInput.New("not enough info for Rootly execution")
-	}
 	if op.ConnectionId == 0 {
 		return errors.BadInput.New("connectionId is invalid")
 	}