From 8f57f06d62954d1e06a1584c7e40463dad8842bd Mon Sep 17 00:00:00 2001 From: Brian Feucht Date: Fri, 29 May 2026 08:45:58 -0700 Subject: [PATCH 1/3] 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> --- backend/plugins/rootly/tasks/incidents_converter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/plugins/rootly/tasks/incidents_converter.go b/backend/plugins/rootly/tasks/incidents_converter.go index 856575d7866..fba7ee6c169 100644 --- a/backend/plugins/rootly/tasks/incidents_converter.go +++ b/backend/plugins/rootly/tasks/incidents_converter.go @@ -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 From a5ec2b39c2f590662ca4f7579bb2cf74f049e5cc Mon Sep 17 00:00:00 2001 From: Brian Feucht Date: Fri, 29 May 2026 08:53:21 -0700 Subject: [PATCH 2/3] 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> --- .../rootly/tasks/incidents_converter.go | 18 ++++++++++------ .../rootly/tasks/incidents_converter_test.go | 21 +++++++++++++++---- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/backend/plugins/rootly/tasks/incidents_converter.go b/backend/plugins/rootly/tasks/incidents_converter.go index fba7ee6c169..daba86f505f 100644 --- a/backend/plugins/rootly/tasks/incidents_converter.go +++ b/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) @@ -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 } diff --git a/backend/plugins/rootly/tasks/incidents_converter_test.go b/backend/plugins/rootly/tasks/incidents_converter_test.go index 6ea99bd2dfe..422f719eb6f 100644 --- a/backend/plugins/rootly/tasks/incidents_converter_test.go +++ b/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) } From f0bb4d18720e9c07b6b2e4bce5bb33025968171e Mon Sep 17 00:00:00 2001 From: Brian Feucht Date: Fri, 29 May 2026 10:26:18 -0700 Subject: [PATCH 3/3] 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> --- backend/plugins/rootly/tasks/incidents_collector.go | 4 +++- backend/plugins/rootly/tasks/incidents_collector_test.go | 7 +++++++ backend/plugins/rootly/tasks/incidents_extractor.go | 9 ++++++--- backend/plugins/rootly/tasks/task_data.go | 9 +++++---- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/backend/plugins/rootly/tasks/incidents_collector.go b/backend/plugins/rootly/tasks/incidents_collector.go index e89bb89fcd7..652801217aa 100644 --- a/backend/plugins/rootly/tasks/incidents_collector.go +++ b/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)) diff --git a/backend/plugins/rootly/tasks/incidents_collector_test.go b/backend/plugins/rootly/tasks/incidents_collector_test.go index eb3c0241581..3456749a409 100644 --- a/backend/plugins/rootly/tasks/incidents_collector_test.go +++ b/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]")) diff --git a/backend/plugins/rootly/tasks/incidents_extractor.go b/backend/plugins/rootly/tasks/incidents_extractor.go index 99aca0946db..3e812e2c79a 100644 --- a/backend/plugins/rootly/tasks/incidents_extractor.go +++ b/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() { diff --git a/backend/plugins/rootly/tasks/task_data.go b/backend/plugins/rootly/tasks/task_data.go index 334483bbe28..c530d1d1706 100644 --- a/backend/plugins/rootly/tasks/task_data.go +++ b/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") }