From 9da0dbdff544a6d90559ec9f4caa27e17231e93b Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:26:47 +0000 Subject: [PATCH 01/10] [AC1][AC3] add client methods to page alert targets and attach alerts to an incident Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/api/client.go | 107 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 103 insertions(+), 4 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index e8764e5..8b1f164 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -2644,7 +2644,25 @@ func (c *Client) EscalateAlertCLI(ctx context.Context, id string, escalationPoli if err != nil { return fmt.Errorf("failed to read response: %w", err) } - if httpResp.StatusCode >= http.StatusBadRequest && httpResp.StatusCode < http.StatusInternalServerError { + return apiStatusError(httpResp.StatusCode, body) +} + +// Notification target types accepted by POST /v1/alerts. +const ( + NotificationTargetGroup = "Group" + NotificationTargetEscalationPolicy = "EscalationPolicy" +) + +// NotificationTarget is a team or escalation policy to page. +type NotificationTarget struct { + Type string `json:"type"` + ID string `json:"id"` +} + +// apiStatusError returns nil for 2xx responses. For 4xx it surfaces the first +// JSON:API error title when present, otherwise just the status code. +func apiStatusError(status int, body []byte) error { + if status >= http.StatusBadRequest && status < http.StatusInternalServerError { var response struct { Errors []struct { Title string `json:"title"` @@ -2653,14 +2671,95 @@ func (c *Client) EscalateAlertCLI(ctx context.Context, id string, escalationPoli if json.Unmarshal(body, &response) == nil && len(response.Errors) > 0 && response.Errors[0].Title != "" { return fmt.Errorf("%s", response.Errors[0].Title) } - return fmt.Errorf("API returned status %d", httpResp.StatusCode) + return fmt.Errorf("API returned status %d", status) } - if httpResp.StatusCode < http.StatusOK || httpResp.StatusCode >= http.StatusMultipleChoices { - return fmt.Errorf("API returned status %d", httpResp.StatusCode) + if status < http.StatusOK || status >= http.StatusMultipleChoices { + return fmt.Errorf("API returned status %d", status) } return nil } +// PageAlertTargetsCLI pages the given targets through POST /v1/alerts. An empty +// alertID creates a new alert; otherwise the existing alert is upserted. It +// returns the ID of the alert that paged the targets. +func (c *Client) PageAlertTargetsCLI(ctx context.Context, alertID, summary, description string, targets []NotificationTarget) (string, error) { + attributes := map[string]interface{}{ + "summary": summary, + "notification_targets": targets, + } + if description != "" { + attributes["description"] = description + } + data := map[string]interface{}{ + "type": "alerts", + "attributes": attributes, + } + if alertID != "" { + data["id"] = alertID + } + bodyBytes, err := json.Marshal(map[string]interface{}{"data": data}) + if err != nil { + return "", fmt.Errorf("failed to marshal request body: %w", err) + } + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.endpoint+"/v1/alerts", strings.NewReader(string(bodyBytes))) + if err != nil { + return "", fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.apiKey) + req.Header.Set("Content-Type", "application/vnd.api+json") + + httpResp, err := c.httpClient.Do(req) + if err != nil { + return "", fmt.Errorf("failed to page team: %w", err) + } + defer func() { _ = httpResp.Body.Close() }() + body, err := io.ReadAll(httpResp.Body) + if err != nil { + return "", fmt.Errorf("failed to read response: %w", err) + } + if err := apiStatusError(httpResp.StatusCode, body); err != nil { + return "", err + } + var response struct { + Data struct { + ID string `json:"id"` + } `json:"data"` + } + if err := json.Unmarshal(body, &response); err != nil { + return "", fmt.Errorf("failed to parse response: %w", err) + } + if response.Data.ID == "" { + return "", fmt.Errorf("failed to parse response: missing alert id") + } + return response.Data.ID, nil +} + +// AttachAlertsToIncidentCLI links existing alerts to an incident. +func (c *Client) AttachAlertsToIncidentCLI(ctx context.Context, incidentID string, alertIDs []string) error { + bodyBytes, err := json.Marshal(map[string]interface{}{"alert_ids": alertIDs}) + if err != nil { + return fmt.Errorf("failed to marshal request body: %w", err) + } + path := fmt.Sprintf("/v1/incidents/%s/alerts", neturl.PathEscape(incidentID)) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, c.endpoint+path, strings.NewReader(string(bodyBytes))) + if err != nil { + return fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.apiKey) + req.Header.Set("Content-Type", "application/vnd.api+json") + + httpResp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("failed to attach alerts to incident: %w", err) + } + defer func() { _ = httpResp.Body.Close() }() + body, err := io.ReadAll(httpResp.Body) + if err != nil { + return fmt.Errorf("failed to read response: %w", err) + } + return apiStatusError(httpResp.StatusCode, body) +} + // ResolveAlertCLI resolves an alert using raw HTTP POST. func (c *Client) ResolveAlertCLI(ctx context.Context, id, resolutionMessage string, resolveIncidents bool) error { // Build URL From 99f2a24565dcad9149163af5417d01808864b3a2 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:26:47 +0000 Subject: [PATCH 02/10] [AC1][AC2] add incidents escalate command Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/cmd/incidents/escalate.go | 75 ++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 internal/cmd/incidents/escalate.go diff --git a/internal/cmd/incidents/escalate.go b/internal/cmd/incidents/escalate.go new file mode 100644 index 0000000..ff7ba80 --- /dev/null +++ b/internal/cmd/incidents/escalate.go @@ -0,0 +1,75 @@ +package incidents + +import ( + "fmt" + "os" + + "github.com/spf13/cobra" + + "github.com/rootlyhq/rootly-cli/internal/api" +) + +var escalateCmd = &cobra.Command{ + Use: "escalate ", + Short: "Page a team for an incident", + Long: `Page a team for an incident. + +Creates an alert that pages the team (and the escalation policy, if given) and links it to the incident. +The incident can be a UUID, slug, or sequential ID such as INC-123.`, + Example: ` # Page a team + rootly incidents escalate INC-123 --team=platform + + # Also page an escalation policy, with a note for the responders + rootly incidents escalate INC-123 --team=platform --escalation-policy= --note="Database is down"`, + Args: cobra.ExactArgs(1), + RunE: runEscalate, +} + +func init() { + escalateCmd.Flags().String("team", "", "Team slug or ID to page") + escalateCmd.Flags().String("escalation-policy", "", "Escalation policy UUID to page as well") + escalateCmd.Flags().String("note", "", "Note for the responders") + _ = escalateCmd.MarkFlagRequired("team") + IncidentsCmd.AddCommand(escalateCmd) +} + +func runEscalate(cmd *cobra.Command, args []string) error { + teamRef, _ := cmd.Flags().GetString("team") + if teamRef == "" { + return fmt.Errorf("--team must not be empty") + } + policyID, _ := cmd.Flags().GetString("escalation-policy") + note, _ := cmd.Flags().GetString("note") + + apiClient, err := getAPIClient() + if err != nil { + return err + } + incident, err := apiClient.GetIncidentByID(cmd.Context(), api.NormalizeIncidentID(args[0])) + if err != nil { + return err + } + team, err := apiClient.GetTeamByID(cmd.Context(), teamRef) + if err != nil { + return fmt.Errorf("unknown team %q: %w", teamRef, err) + } + targets := []api.NotificationTarget{{Type: api.NotificationTargetGroup, ID: team.ID}} + if policyID != "" { + targets = append(targets, api.NotificationTarget{Type: api.NotificationTargetEscalationPolicy, ID: policyID}) + } + + label := incident.ID + if incident.SequentialID != "" { + label = "INC-" + incident.SequentialID + } + summary := fmt.Sprintf("Escalation for %s: %s", label, incident.Title) + alertID, err := apiClient.PageAlertTargetsCLI(cmd.Context(), "", summary, note, targets) + if err != nil { + return fmt.Errorf("failed to page team: %w", err) + } + if err := apiClient.AttachAlertsToIncidentCLI(cmd.Context(), incident.ID, []string{alertID}); err != nil { + return fmt.Errorf("team %s was paged by alert %s but linking it to %s failed: %w", team.Name, alertID, label, err) + } + _, err = fmt.Fprintf(os.Stdout, "Paged team %s for incident %s (alert %s)\n", team.Name, label, alertID) + return err +} From e53130dbed64aeed57923206e90b9d63ffc8c345 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:26:47 +0000 Subject: [PATCH 03/10] [AC3] add --team to alerts escalate Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/cmd/alerts/escalate.go | 51 +++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 5 deletions(-) diff --git a/internal/cmd/alerts/escalate.go b/internal/cmd/alerts/escalate.go index 6bb18ae..fe3f979 100644 --- a/internal/cmd/alerts/escalate.go +++ b/internal/cmd/alerts/escalate.go @@ -5,19 +5,28 @@ import ( "os" "github.com/spf13/cobra" + + "github.com/rootlyhq/rootly-cli/internal/api" ) var escalateCmd = &cobra.Command{ - Use: "escalate ", - Short: "Escalate an alert", - Example: " rootly alerts escalate --escalation-policy= --level=2", - Args: cobra.ExactArgs(1), - RunE: runEscalate, + Use: "escalate ", + Short: "Escalate an alert", + Long: `Escalate an alert. + +With --escalation-policy and/or --level, the alert is escalated through its escalation policy. +With --team, the team (slug or ID) is paged on the existing alert.`, + Example: ` rootly alerts escalate --escalation-policy= --level=2 + rootly alerts escalate --team=platform`, + Args: cobra.ExactArgs(1), + RunE: runEscalate, } func init() { escalateCmd.Flags().String("escalation-policy", "", "Escalation policy UUID") escalateCmd.Flags().Int("level", 0, "Escalation policy level") + escalateCmd.Flags().String("team", "", "Team slug or ID to page") + escalateCmd.MarkFlagsMutuallyExclusive("team", "level") AlertsCmd.AddCommand(escalateCmd) } @@ -27,6 +36,10 @@ func runEscalate(cmd *cobra.Command, args []string) error { value, _ := cmd.Flags().GetString("escalation-policy") policyID = &value } + if cmd.Flags().Changed("team") { + team, _ := cmd.Flags().GetString("team") + return runPageTeam(cmd, args[0], team, policyID) + } var level *int if cmd.Flags().Changed("level") { value, _ := cmd.Flags().GetInt("level") @@ -48,3 +61,31 @@ func runEscalate(cmd *cobra.Command, args []string) error { } return nil } + +// runPageTeam pages a team (and optionally an escalation policy) on an existing alert. +func runPageTeam(cmd *cobra.Command, alertID, teamRef string, policyID *string) error { + if teamRef == "" { + return fmt.Errorf("--team must not be empty") + } + apiClient, err := getAPIClient() + if err != nil { + return err + } + alert, err := apiClient.GetAlertByID(cmd.Context(), alertID) + if err != nil { + return fmt.Errorf("failed to escalate alert: %w", err) + } + team, err := apiClient.GetTeamByID(cmd.Context(), teamRef) + if err != nil { + return fmt.Errorf("unknown team %q: %w", teamRef, err) + } + targets := []api.NotificationTarget{{Type: api.NotificationTargetGroup, ID: team.ID}} + if policyID != nil { + targets = append(targets, api.NotificationTarget{Type: api.NotificationTargetEscalationPolicy, ID: *policyID}) + } + if _, err := apiClient.PageAlertTargetsCLI(cmd.Context(), alert.ID, alert.Summary, "", targets); err != nil { + return fmt.Errorf("failed to page team: %w", err) + } + _, err = fmt.Fprintf(os.Stdout, "Paged team %s on alert %s\n", team.Name, alertID) + return err +} From 108fb68e6cfed167102b51d9aabf4c2609aba17f Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:29:12 +0000 Subject: [PATCH 04/10] [AC4][AC5][AC6] test and document escalate --team; fix incident label prefix Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- README.md | 13 +- internal/api/client_alert_page_test.go | 195 ++++++++++++++++++++++ internal/cmd/alerts/escalate_team_test.go | 174 +++++++++++++++++++ internal/cmd/incidents/escalate.go | 2 +- internal/cmd/incidents/escalate_test.go | 179 ++++++++++++++++++++ 5 files changed, 560 insertions(+), 3 deletions(-) create mode 100644 internal/api/client_alert_page_test.go create mode 100644 internal/cmd/alerts/escalate_team_test.go create mode 100644 internal/cmd/incidents/escalate_test.go diff --git a/README.md b/README.md index e8c9854..f310a9f 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,11 @@ rootly alerts ack rootly alerts resolve rootly alerts escalate rootly alerts escalate --escalation-policy= --level=2 +rootly alerts escalate --team=platform # Page a team on the alert + +# Page a team for an incident (links the page to the incident) +rootly incidents escalate INC-123 --team=platform +rootly incidents escalate INC-123 --team=platform --escalation-policy= --note="Database is down" # Services rootly services list @@ -167,10 +172,14 @@ List a status page's templates with `rootly status-pages templates list --status `status-pages events create` and `status-pages events update` accept incident statuses `investigating`, `identified`, `monitoring`, and `resolved`, plus scheduled-maintenance statuses `scheduled`, `in_progress`, and `completed`. For `status-pages events resolve`, `--status` defaults to `resolved`; use `--status=completed` to complete scheduled maintenance. -### Alert Escalation +### Escalation `rootly alerts escalate ` escalates an alert using its current policy. Optionally pass `--escalation-policy=` and/or `--level=` to select a policy or escalation level; levels must be at least 1. The API's error title is shown when escalation cannot proceed. +Pass `--team=` instead to page a team on the existing alert. `--team` cannot be combined with `--level`; `--escalation-policy` adds that policy as an extra paging target. + +`rootly incidents escalate --team=` pages a team for an incident. The incident can be a UUID, slug, or sequential ID such as `INC-123`. It creates an alert that pages the team and links it to the incident. Optional `--escalation-policy=` pages that policy as well, and `--note=` is sent to the responders as the alert description. An unknown team or incident gives a clear error, and the API's error title is shown otherwise. Paging requires Rootly alerting to be enabled for the organization. + ### Pulse Flags | Flag | Short | Env Var | Description | @@ -232,7 +241,7 @@ rootly incidents list --sort=-created_at | Command | Subcommands | Aliases | |---------|-------------|---------| -| `rootly incidents` | `list`, `get`, `create`, `update`, `delete` | `incident`, `inc` | +| `rootly incidents` | `list`, `get`, `create`, `update`, `delete`, `escalate` | `incident`, `inc` | | `rootly alerts` | `list`, `get`, `create`, `update`, `ack`, `resolve`, `escalate` | `alert`, `alr` | | `rootly form-fields` | `list` | | | `rootly services` | `list`, `get`, `create`, `update`, `delete` | `service`, `svc` | diff --git a/internal/api/client_alert_page_test.go b/internal/api/client_alert_page_test.go new file mode 100644 index 0000000..b3ac13c --- /dev/null +++ b/internal/api/client_alert_page_test.go @@ -0,0 +1,195 @@ +package api + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +func pageTargets() []NotificationTarget { + return []NotificationTarget{ + {Type: NotificationTargetGroup, ID: "team-1"}, + {Type: NotificationTargetEscalationPolicy, ID: "ep-1"}, + } +} + +func TestPageAlertTargetsCLICreate(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/v1/alerts" { + t.Errorf("request = %s %s, want POST /v1/alerts", r.Method, r.URL.Path) + } + if got := r.Header.Get("Authorization"); got != "Bearer test-key" { + t.Errorf("Authorization = %q", got) + } + if got := r.Header.Get("Content-Type"); got != "application/vnd.api+json" { + t.Errorf("Content-Type = %q", got) + } + var body struct { + Data map[string]interface{} `json:"data"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("failed to decode request body: %v", err) + return + } + if _, ok := body.Data["id"]; ok { + t.Errorf("create must not send data.id: %+v", body.Data) + } + attributes := body.Data["attributes"].(map[string]interface{}) + if attributes["summary"] != "Escalation" || attributes["description"] != "db down" { + t.Errorf("attributes = %+v", attributes) + } + targets := attributes["notification_targets"].([]interface{}) + if len(targets) != 2 { + t.Fatalf("targets = %+v", targets) + } + first := targets[0].(map[string]interface{}) + if first["type"] != "Group" || first["id"] != "team-1" { + t.Errorf("first target = %+v", first) + } + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"data":{"id":"alert-new"}}`)) + })) + defer server.Close() + + client := newTestClient(t, server.URL) + id, err := client.PageAlertTargetsCLI(context.Background(), "", "Escalation", "db down", pageTargets()) + if err != nil { + t.Fatalf("PageAlertTargetsCLI returned error: %v", err) + } + if id != "alert-new" { + t.Errorf("alert id = %q, want alert-new", id) + } +} + +func TestPageAlertTargetsCLIUpsertsExistingAlert(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body struct { + Data map[string]interface{} `json:"data"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("failed to decode request body: %v", err) + return + } + if body.Data["id"] != "alert-1" { + t.Errorf("data.id = %v, want alert-1", body.Data["id"]) + } + attributes := body.Data["attributes"].(map[string]interface{}) + if _, ok := attributes["description"]; ok { + t.Errorf("empty description must be omitted: %+v", attributes) + } + _, _ = w.Write([]byte(`{"data":{"id":"alert-1"}}`)) + })) + defer server.Close() + + client := newTestClient(t, server.URL) + id, err := client.PageAlertTargetsCLI(context.Background(), "alert-1", "CPU", "", pageTargets()[:1]) + if err != nil { + t.Fatalf("PageAlertTargetsCLI returned error: %v", err) + } + if id != "alert-1" { + t.Errorf("alert id = %q", id) + } +} + +func TestPageAlertTargetsCLIErrors(t *testing.T) { + tests := []struct { + name string + status int + body string + wantErr string + }{ + {"api title", http.StatusUnprocessableEntity, `{"errors":[{"title":"One or more notification_targets could not be found for this organization."}]}`, "notification_targets could not be found"}, + {"4xx without body", http.StatusForbidden, ``, "API returned status 403"}, + {"5xx", http.StatusBadGateway, ``, "API returned status 502"}, + {"bad json", http.StatusOK, `not json`, "failed to parse response"}, + {"missing id", http.StatusOK, `{"data":{}}`, "missing alert id"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.status) + _, _ = w.Write([]byte(tt.body)) + })) + defer server.Close() + client := newTestClient(t, server.URL) + _, err := client.PageAlertTargetsCLI(context.Background(), "", "s", "", pageTargets()) + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error = %v, want %q", err, tt.wantErr) + } + }) + } +} + +func TestPageAlertTargetsCLITransportError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + client := newTestClient(t, server.URL) + server.Close() + _, err := client.PageAlertTargetsCLI(context.Background(), "", "s", "", pageTargets()) + if err == nil || !strings.Contains(err.Error(), "failed to page team") { + t.Fatalf("error = %v", err) + } +} + +func TestAttachAlertsToIncidentCLI(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost || r.URL.Path != "/v1/incidents/inc-1/alerts" { + t.Errorf("request = %s %s, want POST /v1/incidents/inc-1/alerts", r.Method, r.URL.Path) + } + var body struct { + AlertIDs []string `json:"alert_ids"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("failed to decode request body: %v", err) + return + } + if len(body.AlertIDs) != 1 || body.AlertIDs[0] != "alert-1" { + t.Errorf("alert_ids = %v", body.AlertIDs) + } + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + client := newTestClient(t, server.URL) + if err := client.AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-1"}); err != nil { + t.Fatalf("AttachAlertsToIncidentCLI returned error: %v", err) + } +} + +func TestAttachAlertsToIncidentCLIErrors(t *testing.T) { + tests := []struct { + name string + status int + body string + wantErr string + }{ + {"api title", http.StatusNotFound, `{"errors":[{"title":"Incident not found"}]}`, "Incident not found"}, + {"5xx", http.StatusInternalServerError, ``, "API returned status 500"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(tt.status) + _, _ = w.Write([]byte(tt.body)) + })) + defer server.Close() + client := newTestClient(t, server.URL) + err := client.AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"a"}) + if err == nil || !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("error = %v, want %q", err, tt.wantErr) + } + }) + } +} + +func TestAttachAlertsToIncidentCLITransportError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + client := newTestClient(t, server.URL) + server.Close() + err := client.AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"a"}) + if err == nil || !strings.Contains(err.Error(), "failed to attach alerts") { + t.Fatalf("error = %v", err) + } +} diff --git a/internal/cmd/alerts/escalate_team_test.go b/internal/cmd/alerts/escalate_team_test.go new file mode 100644 index 0000000..a8683de --- /dev/null +++ b/internal/cmd/alerts/escalate_team_test.go @@ -0,0 +1,174 @@ +package alerts + +import ( + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +const teamResponse = `{"data":{"id":"team-uuid","attributes":{"name":"Platform","slug":"platform"}}}` + +func newTeamEscalateCmd(t *testing.T, flags map[string]string) *cobra.Command { + t.Helper() + cmd := newTestCmd() + cmd.Flags().String("escalation-policy", "", "") + cmd.Flags().Int("level", 0, "") + cmd.Flags().String("team", "", "") + for name, value := range flags { + if err := cmd.Flags().Set(name, value); err != nil { + t.Fatal(err) + } + } + return cmd +} + +func TestRunEscalateTeamPagesExistingAlert(t *testing.T) { + var paged map[string]interface{} + setupTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && r.URL.Path == "/v1/alerts/ALR-42": + _, _ = w.Write([]byte(alertGetResponse())) + case r.Method == http.MethodGet && r.URL.Path == "/v1/teams/platform": + _, _ = w.Write([]byte(teamResponse)) + case r.Method == http.MethodPost && r.URL.Path == "/v1/alerts": + if err := json.NewDecoder(r.Body).Decode(&paged); err != nil { + t.Errorf("failed to decode request body: %v", err) + } + _, _ = w.Write([]byte(`{"data":{"id":"alert-1"}}`)) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + }) + cmd := newTeamEscalateCmd(t, map[string]string{"team": "platform"}) + output := captureStdout(t, func() { + if err := runEscalate(cmd, []string{"ALR-42"}); err != nil { + t.Fatalf("runEscalate returned error: %v", err) + } + }) + if !strings.Contains(output, "Paged team Platform on alert ALR-42") { + t.Errorf("output = %q", output) + } + data := paged["data"].(map[string]interface{}) + if data["id"] != "alert-1" { + t.Errorf("data.id = %v, want alert-1", data["id"]) + } + attributes := data["attributes"].(map[string]interface{}) + if attributes["summary"] != "High CPU usage" { + t.Errorf("summary = %v", attributes["summary"]) + } + targets := attributes["notification_targets"].([]interface{}) + if len(targets) != 1 || targets[0].(map[string]interface{})["id"] != "team-uuid" || targets[0].(map[string]interface{})["type"] != "Group" { + t.Errorf("targets = %+v", targets) + } +} + +func TestRunEscalateTeamWithPolicyPagesBothTargets(t *testing.T) { + var targets []interface{} + setupTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/v1/alerts/"): + _, _ = w.Write([]byte(alertGetResponse())) + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/v1/teams/"): + _, _ = w.Write([]byte(teamResponse)) + default: + var body map[string]interface{} + _ = json.NewDecoder(r.Body).Decode(&body) + targets = body["data"].(map[string]interface{})["attributes"].(map[string]interface{})["notification_targets"].([]interface{}) + _, _ = w.Write([]byte(`{"data":{"id":"alert-1"}}`)) + } + }) + cmd := newTeamEscalateCmd(t, map[string]string{"team": "platform", "escalation-policy": "ep-1"}) + captureStdout(t, func() { + if err := runEscalate(cmd, []string{"ALR-42"}); err != nil { + t.Fatalf("runEscalate returned error: %v", err) + } + }) + if len(targets) != 2 || targets[1].(map[string]interface{})["type"] != "EscalationPolicy" || targets[1].(map[string]interface{})["id"] != "ep-1" { + t.Errorf("targets = %+v", targets) + } +} + +func TestRunEscalateTeamRejectsEmptyTeam(t *testing.T) { + setupTestServer(t, func(_ http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + }) + cmd := newTeamEscalateCmd(t, map[string]string{"team": ""}) + err := runEscalate(cmd, []string{"ALR-42"}) + if err == nil || !strings.Contains(err.Error(), "--team must not be empty") { + t.Fatalf("error = %v", err) + } +} + +func TestRunEscalateTeamUnknownTeam(t *testing.T) { + setupTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/v1/alerts/") { + _, _ = w.Write([]byte(alertGetResponse())) + return + } + w.WriteHeader(http.StatusNotFound) + }) + cmd := newTeamEscalateCmd(t, map[string]string{"team": "nope"}) + err := runEscalate(cmd, []string{"ALR-42"}) + if err == nil || !strings.Contains(err.Error(), `unknown team "nope"`) { + t.Fatalf("error = %v", err) + } +} + +func TestRunEscalateTeamUnknownAlert(t *testing.T) { + setupTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + cmd := newTeamEscalateCmd(t, map[string]string{"team": "platform"}) + err := runEscalate(cmd, []string{"ALR-404"}) + if err == nil || !strings.Contains(err.Error(), "alert not found") { + t.Fatalf("error = %v", err) + } +} + +func TestRunEscalateTeamSurfacesPagingError(t *testing.T) { + setupTestServer(t, func(w http.ResponseWriter, r *http.Request) { + switch { + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/v1/alerts/"): + _, _ = w.Write([]byte(alertGetResponse())) + case r.Method == http.MethodGet: + _, _ = w.Write([]byte(teamResponse)) + default: + w.WriteHeader(http.StatusUnprocessableEntity) + _, _ = w.Write([]byte(`{"errors":[{"title":"Alerting is not enabled for this organization."}]}`)) + } + }) + cmd := newTeamEscalateCmd(t, map[string]string{"team": "platform"}) + err := runEscalate(cmd, []string{"ALR-42"}) + if err == nil || !strings.Contains(err.Error(), "Alerting is not enabled for this organization.") { + t.Fatalf("error = %v", err) + } +} + +func TestEscalateCmdTeamExcludesLevel(t *testing.T) { + setupTestServer(t, func(_ http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + }) + t.Cleanup(func() { + for _, name := range []string{"team", "level"} { + f := escalateCmd.Flags().Lookup(name) + _ = f.Value.Set(f.DefValue) + f.Changed = false + } + }) + if err := escalateCmd.ParseFlags([]string{"--team=platform", "--level=2"}); err != nil { + t.Fatal(err) + } + err := escalateCmd.ValidateFlagGroups() + if err == nil || !strings.Contains(err.Error(), "none of the others can be") { + t.Fatalf("error = %v, want mutual exclusion", err) + } +} + +func TestEscalateHelpDocumentsTeam(t *testing.T) { + if !strings.Contains(escalateCmd.Example, "--team=") || !strings.Contains(escalateCmd.Long, "--team") { + t.Errorf("help does not document --team") + } +} diff --git a/internal/cmd/incidents/escalate.go b/internal/cmd/incidents/escalate.go index ff7ba80..5628ec0 100644 --- a/internal/cmd/incidents/escalate.go +++ b/internal/cmd/incidents/escalate.go @@ -60,7 +60,7 @@ func runEscalate(cmd *cobra.Command, args []string) error { label := incident.ID if incident.SequentialID != "" { - label = "INC-" + incident.SequentialID + label = incident.SequentialID } summary := fmt.Sprintf("Escalation for %s: %s", label, incident.Title) alertID, err := apiClient.PageAlertTargetsCLI(cmd.Context(), "", summary, note, targets) diff --git a/internal/cmd/incidents/escalate_test.go b/internal/cmd/incidents/escalate_test.go new file mode 100644 index 0000000..c48571a --- /dev/null +++ b/internal/cmd/incidents/escalate_test.go @@ -0,0 +1,179 @@ +package incidents + +import ( + "encoding/json" + "net/http" + "strings" + "testing" + + "github.com/spf13/cobra" +) + +const escalateTeamResponse = `{"data":{"id":"team-uuid","attributes":{"name":"Platform","slug":"platform"}}}` + +func newEscalateCmd(t *testing.T, flags map[string]string) *cobra.Command { + t.Helper() + cmd := newTestCmd() + cmd.Flags().String("team", "", "") + cmd.Flags().String("escalation-policy", "", "") + cmd.Flags().String("note", "", "") + for name, value := range flags { + if err := cmd.Flags().Set(name, value); err != nil { + t.Fatal(err) + } + } + return cmd +} + +type escalateRecorder struct { + paged map[string]interface{} + attached map[string]interface{} + paths []string +} + +func (rec *escalateRecorder) handler(t *testing.T, pageStatus, attachStatus int) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + rec.paths = append(rec.paths, r.Method+" "+r.URL.Path) + switch { + case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/v1/incidents/"): + _, _ = w.Write([]byte(getResponse())) + case r.Method == http.MethodGet && r.URL.Path == "/v1/teams/platform": + _, _ = w.Write([]byte(escalateTeamResponse)) + case r.Method == http.MethodGet: + w.WriteHeader(http.StatusNotFound) + case r.Method == http.MethodPost && r.URL.Path == "/v1/alerts": + _ = json.NewDecoder(r.Body).Decode(&rec.paged) + if pageStatus != http.StatusOK { + w.WriteHeader(pageStatus) + _, _ = w.Write([]byte(`{"errors":[{"title":"One or more notification_targets could not be found for this organization."}]}`)) + return + } + _, _ = w.Write([]byte(`{"data":{"id":"alert-new"}}`)) + case r.Method == http.MethodPost && r.URL.Path == "/v1/incidents/inc-1/alerts": + _ = json.NewDecoder(r.Body).Decode(&rec.attached) + w.WriteHeader(attachStatus) + default: + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + } + } +} + +func TestRunEscalatePagesTeamAndLinksIncident(t *testing.T) { + rec := &escalateRecorder{} + setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusOK)) + cmd := newEscalateCmd(t, map[string]string{"team": "platform"}) + output := captureStdout(t, func() { + if err := runEscalate(cmd, []string{"INC-42"}); err != nil { + t.Fatalf("runEscalate returned error: %v", err) + } + }) + if rec.paths[0] != "GET /v1/incidents/42" { + t.Errorf("first request = %q, want INC-42 normalized to 42", rec.paths[0]) + } + data := rec.paged["data"].(map[string]interface{}) + if _, ok := data["id"]; ok { + t.Errorf("a new alert must not send data.id: %+v", data) + } + attributes := data["attributes"].(map[string]interface{}) + if !strings.Contains(attributes["summary"].(string), "INC-42") || !strings.Contains(attributes["summary"].(string), "DB outage") { + t.Errorf("summary = %v", attributes["summary"]) + } + if _, ok := attributes["description"]; ok { + t.Errorf("description must be omitted without --note: %+v", attributes) + } + targets := attributes["notification_targets"].([]interface{}) + if len(targets) != 1 || targets[0].(map[string]interface{})["type"] != "Group" || targets[0].(map[string]interface{})["id"] != "team-uuid" { + t.Errorf("targets = %+v", targets) + } + ids := rec.attached["alert_ids"].([]interface{}) + if len(ids) != 1 || ids[0] != "alert-new" { + t.Errorf("alert_ids = %v", ids) + } + if !strings.Contains(output, "Paged team Platform for incident INC-42 (alert alert-new)") { + t.Errorf("output = %q", output) + } +} + +func TestRunEscalateWithPolicyAndNote(t *testing.T) { + rec := &escalateRecorder{} + setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusOK)) + cmd := newEscalateCmd(t, map[string]string{"team": "platform", "escalation-policy": "ep-1", "note": "db down"}) + captureStdout(t, func() { + if err := runEscalate(cmd, []string{"inc-1"}); err != nil { + t.Fatalf("runEscalate returned error: %v", err) + } + }) + attributes := rec.paged["data"].(map[string]interface{})["attributes"].(map[string]interface{}) + if attributes["description"] != "db down" { + t.Errorf("description = %v", attributes["description"]) + } + targets := attributes["notification_targets"].([]interface{}) + if len(targets) != 2 || targets[1].(map[string]interface{})["type"] != "EscalationPolicy" || targets[1].(map[string]interface{})["id"] != "ep-1" { + t.Errorf("targets = %+v", targets) + } +} + +func TestRunEscalateRejectsEmptyTeam(t *testing.T) { + setupTestServer(t, func(_ http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + }) + err := runEscalate(newEscalateCmd(t, nil), []string{"INC-42"}) + if err == nil || !strings.Contains(err.Error(), "--team must not be empty") { + t.Fatalf("error = %v", err) + } +} + +func TestEscalateCmdRequiresTeam(t *testing.T) { + setupTestServer(t, func(_ http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) + }) + err := escalateCmd.ValidateRequiredFlags() + if err == nil || !strings.Contains(err.Error(), `required flag(s) "team" not set`) { + t.Fatalf("error = %v", err) + } +} + +func TestRunEscalateUnknownIncident(t *testing.T) { + setupTestServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNotFound) + }) + err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform"}), []string{"INC-404"}) + if err == nil || !strings.Contains(err.Error(), "not found") { + t.Fatalf("error = %v", err) + } +} + +func TestRunEscalateUnknownTeam(t *testing.T) { + rec := &escalateRecorder{} + setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusOK)) + err := runEscalate(newEscalateCmd(t, map[string]string{"team": "nope"}), []string{"INC-42"}) + if err == nil || !strings.Contains(err.Error(), `unknown team "nope"`) { + t.Fatalf("error = %v", err) + } +} + +func TestRunEscalateSurfacesPagingError(t *testing.T) { + rec := &escalateRecorder{} + setupTestServer(t, rec.handler(t, http.StatusUnprocessableEntity, http.StatusOK)) + err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform"}), []string{"INC-42"}) + if err == nil || !strings.Contains(err.Error(), "notification_targets could not be found") { + t.Fatalf("error = %v", err) + } +} + +func TestRunEscalateLinkFailureNamesCreatedAlert(t *testing.T) { + rec := &escalateRecorder{} + setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusInternalServerError)) + err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform"}), []string{"INC-42"}) + if err == nil || !strings.Contains(err.Error(), "was paged by alert alert-new") { + t.Fatalf("error = %v", err) + } +} + +func TestEscalateHelpDocumentsFlags(t *testing.T) { + for _, want := range []string{"--team=", "--escalation-policy=", "--note=", "INC-123"} { + if !strings.Contains(escalateCmd.Example, want) { + t.Errorf("example does not mention %s", want) + } + } +} From ee81b4f40e53995b0022bc17a8ccc1f36d226c91 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:36:43 +0000 Subject: [PATCH 05/10] [AC1][AC3][AC4] fix escalate against the server contract: upsert id and alert_ids under attributes, keep linked alerts, no stale summary, --link-alert retry Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- README.md | 2 +- internal/api/client.go | 72 ++++++++++++++++++++-- internal/api/client_alert_page_test.go | 73 ++++++++++++++++++----- internal/cmd/alerts/escalate.go | 2 +- internal/cmd/alerts/escalate_team_test.go | 10 ++-- internal/cmd/incidents/escalate.go | 14 +++-- internal/cmd/incidents/escalate_test.go | 46 +++++++++++++- 7 files changed, 187 insertions(+), 32 deletions(-) diff --git a/README.md b/README.md index f310a9f..b9328de 100644 --- a/README.md +++ b/README.md @@ -178,7 +178,7 @@ List a status page's templates with `rootly status-pages templates list --status Pass `--team=` instead to page a team on the existing alert. `--team` cannot be combined with `--level`; `--escalation-policy` adds that policy as an extra paging target. -`rootly incidents escalate --team=` pages a team for an incident. The incident can be a UUID, slug, or sequential ID such as `INC-123`. It creates an alert that pages the team and links it to the incident. Optional `--escalation-policy=` pages that policy as well, and `--note=` is sent to the responders as the alert description. An unknown team or incident gives a clear error, and the API's error title is shown otherwise. Paging requires Rootly alerting to be enabled for the organization. +`rootly incidents escalate --team=` pages a team for an incident. The incident can be a UUID, slug, or sequential ID such as `INC-123`. It creates an alert that pages the team and links it to the incident. Optional `--escalation-policy=` pages that policy as well, and `--note=` is sent to the responders as the alert description. If the team was paged but linking the alert failed, the error prints the alert ID; retry only the link with `--link-alert=` so the team is not paged twice. Alerts already linked to the incident are kept. An unknown team or incident gives a clear error, and the API's error title is shown otherwise. Paging requires Rootly alerting to be enabled for the organization. ### Pulse Flags diff --git a/internal/api/client.go b/internal/api/client.go index 8b1f164..2dbdbf8 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -2684,9 +2684,13 @@ func apiStatusError(status int, body []byte) error { // returns the ID of the alert that paged the targets. func (c *Client) PageAlertTargetsCLI(ctx context.Context, alertID, summary, description string, targets []NotificationTarget) (string, error) { attributes := map[string]interface{}{ - "summary": summary, "notification_targets": targets, } + // An existing alert keeps its summary: sending a stale one would overwrite + // concurrent edits. The server needs it only to create the alert. + if alertID == "" { + attributes["summary"] = summary + } if description != "" { attributes["description"] = description } @@ -2695,7 +2699,8 @@ func (c *Client) PageAlertTargetsCLI(ctx context.Context, alertID, summary, desc "attributes": attributes, } if alertID != "" { - data["id"] = alertID + // The server reads the upsert id from data.attributes only. + attributes["id"] = alertID } bodyBytes, err := json.Marshal(map[string]interface{}{"data": data}) if err != nil { @@ -2734,9 +2739,68 @@ func (c *Client) PageAlertTargetsCLI(ctx context.Context, alertID, summary, desc return response.Data.ID, nil } -// AttachAlertsToIncidentCLI links existing alerts to an incident. +// listIncidentAlertIDsCLI returns the IDs of every alert already linked to an incident. +func (c *Client) listIncidentAlertIDsCLI(ctx context.Context, incidentID string) ([]string, error) { + const pageSize = 100 + var ids []string + for page := 1; ; page++ { + path := fmt.Sprintf("/v1/incidents/%s/alerts?page[number]=%d&page[size]=%d", neturl.PathEscape(incidentID), page, pageSize) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint+path, nil) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.apiKey) + req.Header.Set("Content-Type", "application/vnd.api+json") + httpResp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to list incident alerts: %w", err) + } + body, err := io.ReadAll(httpResp.Body) + _ = httpResp.Body.Close() + if err != nil { + return nil, fmt.Errorf("failed to read response: %w", err) + } + if err := apiStatusError(httpResp.StatusCode, body); err != nil { + return nil, err + } + var response struct { + Data []struct { + ID string `json:"id"` + } `json:"data"` + } + if err := json.Unmarshal(body, &response); err != nil { + return nil, fmt.Errorf("failed to parse response: %w", err) + } + for _, a := range response.Data { + ids = append(ids, a.ID) + } + if len(response.Data) < pageSize { + return ids, nil + } + } +} + +// AttachAlertsToIncidentCLI links alerts to an incident. The endpoint replaces +// the incident's alert set, so the already linked alerts are sent along. func (c *Client) AttachAlertsToIncidentCLI(ctx context.Context, incidentID string, alertIDs []string) error { - bodyBytes, err := json.Marshal(map[string]interface{}{"alert_ids": alertIDs}) + existing, err := c.listIncidentAlertIDsCLI(ctx, incidentID) + if err != nil { + return err + } + seen := make(map[string]bool, len(existing)) + merged := make([]string, 0, len(existing)+len(alertIDs)) + for _, id := range append(existing, alertIDs...) { + if !seen[id] { + seen[id] = true + merged = append(merged, id) + } + } + bodyBytes, err := json.Marshal(map[string]interface{}{ + "data": map[string]interface{}{ + "type": "incidents", + "attributes": map[string]interface{}{"alert_ids": merged}, + }, + }) if err != nil { return fmt.Errorf("failed to marshal request body: %w", err) } diff --git a/internal/api/client_alert_page_test.go b/internal/api/client_alert_page_test.go index b3ac13c..6e39986 100644 --- a/internal/api/client_alert_page_test.go +++ b/internal/api/client_alert_page_test.go @@ -73,10 +73,16 @@ func TestPageAlertTargetsCLIUpsertsExistingAlert(t *testing.T) { t.Errorf("failed to decode request body: %v", err) return } - if body.Data["id"] != "alert-1" { - t.Errorf("data.id = %v, want alert-1", body.Data["id"]) - } attributes := body.Data["attributes"].(map[string]interface{}) + if attributes["id"] != "alert-1" { + t.Errorf("attributes.id = %v, want alert-1 (the server reads the upsert id from attributes)", attributes["id"]) + } + if _, ok := body.Data["id"]; ok { + t.Errorf("data.id is ignored by the server and must not be relied on: %+v", body.Data) + } + if _, ok := attributes["summary"]; ok { + t.Errorf("existing alert must not be sent a summary: %+v", attributes) + } if _, ok := attributes["description"]; ok { t.Errorf("empty description must be omitted: %+v", attributes) } @@ -134,28 +140,66 @@ func TestPageAlertTargetsCLITransportError(t *testing.T) { } func TestAttachAlertsToIncidentCLI(t *testing.T) { + var posted struct { + Data struct { + Attributes struct { + AlertIDs []string `json:"alert_ids"` + } `json:"attributes"` + } `json:"data"` + } server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodPost || r.URL.Path != "/v1/incidents/inc-1/alerts" { - t.Errorf("request = %s %s, want POST /v1/incidents/inc-1/alerts", r.Method, r.URL.Path) - } - var body struct { - AlertIDs []string `json:"alert_ids"` + if r.URL.Path != "/v1/incidents/inc-1/alerts" { + t.Errorf("path = %s", r.URL.Path) } - if err := json.NewDecoder(r.Body).Decode(&body); err != nil { - t.Errorf("failed to decode request body: %v", err) + if r.Method == http.MethodGet { + _, _ = w.Write([]byte(`{"data":[{"id":"alert-old"},{"id":"alert-1"}]}`)) return } - if len(body.AlertIDs) != 1 || body.AlertIDs[0] != "alert-1" { - t.Errorf("alert_ids = %v", body.AlertIDs) + if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { + t.Errorf("failed to decode request body: %v", err) } w.WriteHeader(http.StatusOK) })) defer server.Close() client := newTestClient(t, server.URL) - if err := client.AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-1"}); err != nil { + if err := client.AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-1", "alert-2"}); err != nil { t.Fatalf("AttachAlertsToIncidentCLI returned error: %v", err) } + got := strings.Join(posted.Data.Attributes.AlertIDs, ",") + if got != "alert-old,alert-1,alert-2" { + t.Errorf("data.attributes.alert_ids = %s, want existing alerts kept, deduplicated, new appended", got) + } +} + +func TestListIncidentAlertIDsCLIPaginates(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("page[number]") == "1" { + items := make([]string, 100) + for i := range items { + items[i] = `{"id":"a` + strings.Repeat("x", i) + `"}` + } + _, _ = w.Write([]byte(`{"data":[` + strings.Join(items, ",") + `]}`)) + return + } + _, _ = w.Write([]byte(`{"data":[{"id":"last"}]}`)) + })) + defer server.Close() + ids, err := newTestClient(t, server.URL).listIncidentAlertIDsCLI(context.Background(), "inc-1") + if err != nil || len(ids) != 101 || ids[100] != "last" { + t.Fatalf("ids = %d, err = %v", len(ids), err) + } +} + +func TestListIncidentAlertIDsCLIBadJSON(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`nope`)) + })) + defer server.Close() + _, err := newTestClient(t, server.URL).listIncidentAlertIDsCLI(context.Background(), "inc-1") + if err == nil || !strings.Contains(err.Error(), "failed to parse response") { + t.Fatalf("error = %v", err) + } } func TestAttachAlertsToIncidentCLIErrors(t *testing.T) { @@ -168,6 +212,7 @@ func TestAttachAlertsToIncidentCLIErrors(t *testing.T) { {"api title", http.StatusNotFound, `{"errors":[{"title":"Incident not found"}]}`, "Incident not found"}, {"5xx", http.StatusInternalServerError, ``, "API returned status 500"}, } + // The same status on the initial listing and on the update both fail. for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -189,7 +234,7 @@ func TestAttachAlertsToIncidentCLITransportError(t *testing.T) { client := newTestClient(t, server.URL) server.Close() err := client.AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"a"}) - if err == nil || !strings.Contains(err.Error(), "failed to attach alerts") { + if err == nil || !strings.Contains(err.Error(), "failed to list incident alerts") { t.Fatalf("error = %v", err) } } diff --git a/internal/cmd/alerts/escalate.go b/internal/cmd/alerts/escalate.go index fe3f979..8904eea 100644 --- a/internal/cmd/alerts/escalate.go +++ b/internal/cmd/alerts/escalate.go @@ -83,7 +83,7 @@ func runPageTeam(cmd *cobra.Command, alertID, teamRef string, policyID *string) if policyID != nil { targets = append(targets, api.NotificationTarget{Type: api.NotificationTargetEscalationPolicy, ID: *policyID}) } - if _, err := apiClient.PageAlertTargetsCLI(cmd.Context(), alert.ID, alert.Summary, "", targets); err != nil { + if _, err := apiClient.PageAlertTargetsCLI(cmd.Context(), alert.ID, "", "", targets); err != nil { return fmt.Errorf("failed to page team: %w", err) } _, err = fmt.Fprintf(os.Stdout, "Paged team %s on alert %s\n", team.Name, alertID) diff --git a/internal/cmd/alerts/escalate_team_test.go b/internal/cmd/alerts/escalate_team_test.go index a8683de..78311b1 100644 --- a/internal/cmd/alerts/escalate_team_test.go +++ b/internal/cmd/alerts/escalate_team_test.go @@ -52,12 +52,12 @@ func TestRunEscalateTeamPagesExistingAlert(t *testing.T) { t.Errorf("output = %q", output) } data := paged["data"].(map[string]interface{}) - if data["id"] != "alert-1" { - t.Errorf("data.id = %v, want alert-1", data["id"]) - } attributes := data["attributes"].(map[string]interface{}) - if attributes["summary"] != "High CPU usage" { - t.Errorf("summary = %v", attributes["summary"]) + if attributes["id"] != "alert-1" { + t.Errorf("attributes.id = %v, want alert-1", attributes["id"]) + } + if _, ok := attributes["summary"]; ok { + t.Errorf("summary must not be rewritten from the earlier read: %+v", attributes) } targets := attributes["notification_targets"].([]interface{}) if len(targets) != 1 || targets[0].(map[string]interface{})["id"] != "team-uuid" || targets[0].(map[string]interface{})["type"] != "Group" { diff --git a/internal/cmd/incidents/escalate.go b/internal/cmd/incidents/escalate.go index 5628ec0..b18b13e 100644 --- a/internal/cmd/incidents/escalate.go +++ b/internal/cmd/incidents/escalate.go @@ -29,6 +29,7 @@ func init() { escalateCmd.Flags().String("team", "", "Team slug or ID to page") escalateCmd.Flags().String("escalation-policy", "", "Escalation policy UUID to page as well") escalateCmd.Flags().String("note", "", "Note for the responders") + escalateCmd.Flags().String("link-alert", "", "Only link this already created alert to the incident, without paging again (retry after a failed link)") _ = escalateCmd.MarkFlagRequired("team") IncidentsCmd.AddCommand(escalateCmd) } @@ -62,13 +63,16 @@ func runEscalate(cmd *cobra.Command, args []string) error { if incident.SequentialID != "" { label = incident.SequentialID } - summary := fmt.Sprintf("Escalation for %s: %s", label, incident.Title) - alertID, err := apiClient.PageAlertTargetsCLI(cmd.Context(), "", summary, note, targets) - if err != nil { - return fmt.Errorf("failed to page team: %w", err) + alertID, _ := cmd.Flags().GetString("link-alert") + if alertID == "" { + summary := fmt.Sprintf("Escalation for %s: %s", label, incident.Title) + alertID, err = apiClient.PageAlertTargetsCLI(cmd.Context(), "", summary, note, targets) + if err != nil { + return fmt.Errorf("failed to page team: %w", err) + } } if err := apiClient.AttachAlertsToIncidentCLI(cmd.Context(), incident.ID, []string{alertID}); err != nil { - return fmt.Errorf("team %s was paged by alert %s but linking it to %s failed: %w", team.Name, alertID, label, err) + return fmt.Errorf("team %s was paged by alert %s but linking it to %s failed; retry the link without paging again with --link-alert=%s: %w", team.Name, alertID, label, alertID, err) } _, err = fmt.Fprintf(os.Stdout, "Paged team %s for incident %s (alert %s)\n", team.Name, label, alertID) return err diff --git a/internal/cmd/incidents/escalate_test.go b/internal/cmd/incidents/escalate_test.go index c48571a..3b5be98 100644 --- a/internal/cmd/incidents/escalate_test.go +++ b/internal/cmd/incidents/escalate_test.go @@ -17,6 +17,7 @@ func newEscalateCmd(t *testing.T, flags map[string]string) *cobra.Command { cmd.Flags().String("team", "", "") cmd.Flags().String("escalation-policy", "", "") cmd.Flags().String("note", "", "") + cmd.Flags().String("link-alert", "", "") for name, value := range flags { if err := cmd.Flags().Set(name, value); err != nil { t.Fatal(err) @@ -29,12 +30,19 @@ type escalateRecorder struct { paged map[string]interface{} attached map[string]interface{} paths []string + existing string // JSON array body of alerts already linked to the incident } func (rec *escalateRecorder) handler(t *testing.T, pageStatus, attachStatus int) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { rec.paths = append(rec.paths, r.Method+" "+r.URL.Path) switch { + case r.Method == http.MethodGet && r.URL.Path == "/v1/incidents/inc-1/alerts": + existing := rec.existing + if existing == "" { + existing = "[]" + } + _, _ = w.Write([]byte(`{"data":` + existing + `}`)) case r.Method == http.MethodGet && strings.HasPrefix(r.URL.Path, "/v1/incidents/"): _, _ = w.Write([]byte(getResponse())) case r.Method == http.MethodGet && r.URL.Path == "/v1/teams/platform": @@ -85,7 +93,7 @@ func TestRunEscalatePagesTeamAndLinksIncident(t *testing.T) { if len(targets) != 1 || targets[0].(map[string]interface{})["type"] != "Group" || targets[0].(map[string]interface{})["id"] != "team-uuid" { t.Errorf("targets = %+v", targets) } - ids := rec.attached["alert_ids"].([]interface{}) + ids := rec.attached["data"].(map[string]interface{})["attributes"].(map[string]interface{})["alert_ids"].([]interface{}) if len(ids) != 1 || ids[0] != "alert-new" { t.Errorf("alert_ids = %v", ids) } @@ -165,11 +173,45 @@ func TestRunEscalateLinkFailureNamesCreatedAlert(t *testing.T) { rec := &escalateRecorder{} setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusInternalServerError)) err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform"}), []string{"INC-42"}) - if err == nil || !strings.Contains(err.Error(), "was paged by alert alert-new") { + if err == nil || !strings.Contains(err.Error(), "was paged by alert alert-new") || !strings.Contains(err.Error(), "--link-alert=alert-new") { t.Fatalf("error = %v", err) } } +func TestRunEscalateKeepsAlreadyLinkedAlerts(t *testing.T) { + rec := &escalateRecorder{existing: `[{"id":"alert-old"}]`} + setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusOK)) + captureStdout(t, func() { + if err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform"}), []string{"INC-42"}); err != nil { + t.Fatalf("runEscalate returned error: %v", err) + } + }) + ids := rec.attached["data"].(map[string]interface{})["attributes"].(map[string]interface{})["alert_ids"].([]interface{}) + if len(ids) != 2 || ids[0] != "alert-old" || ids[1] != "alert-new" { + t.Errorf("alert_ids = %v, want the existing alert kept and the new one appended", ids) + } +} + +func TestRunEscalateLinkAlertRetriesWithoutPagingAgain(t *testing.T) { + rec := &escalateRecorder{} + setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusOK)) + output := captureStdout(t, func() { + if err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform", "link-alert": "alert-new"}), []string{"INC-42"}); err != nil { + t.Fatalf("runEscalate returned error: %v", err) + } + }) + if rec.paged != nil { + t.Errorf("--link-alert must not page again: %+v", rec.paged) + } + ids := rec.attached["data"].(map[string]interface{})["attributes"].(map[string]interface{})["alert_ids"].([]interface{}) + if len(ids) != 1 || ids[0] != "alert-new" { + t.Errorf("alert_ids = %v", ids) + } + if !strings.Contains(output, "alert alert-new") { + t.Errorf("output = %q", output) + } +} + func TestEscalateHelpDocumentsFlags(t *testing.T) { for _, want := range []string{"--team=", "--escalation-policy=", "--note=", "INC-123"} { if !strings.Contains(escalateCmd.Example, want) { From eafe0738aa44abeb86642a89aab47c59574e0072 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:41:58 +0000 Subject: [PATCH 06/10] [AC1] use http.NoBody for the GET in the alert lookup (gocritic httpNoBody) Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/api/client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/client.go b/internal/api/client.go index 2dbdbf8..fdeb78d 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -2745,7 +2745,7 @@ func (c *Client) listIncidentAlertIDsCLI(ctx context.Context, incidentID string) var ids []string for page := 1; ; page++ { path := fmt.Sprintf("/v1/incidents/%s/alerts?page[number]=%d&page[size]=%d", neturl.PathEscape(incidentID), page, pageSize) - req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint+path, nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, c.endpoint+path, http.NoBody) if err != nil { return nil, fmt.Errorf("failed to create request: %w", err) } From be7c2dc74f9e23e92fd442d30c9f03ba32ed68bb Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:42:49 +0000 Subject: [PATCH 07/10] [AC1] let --link-alert retry link without --team or a team lookup Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/cmd/incidents/escalate.go | 33 ++++++++++++++----------- internal/cmd/incidents/escalate_test.go | 10 ++++---- 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/internal/cmd/incidents/escalate.go b/internal/cmd/incidents/escalate.go index b18b13e..234924c 100644 --- a/internal/cmd/incidents/escalate.go +++ b/internal/cmd/incidents/escalate.go @@ -30,13 +30,13 @@ func init() { escalateCmd.Flags().String("escalation-policy", "", "Escalation policy UUID to page as well") escalateCmd.Flags().String("note", "", "Note for the responders") escalateCmd.Flags().String("link-alert", "", "Only link this already created alert to the incident, without paging again (retry after a failed link)") - _ = escalateCmd.MarkFlagRequired("team") IncidentsCmd.AddCommand(escalateCmd) } func runEscalate(cmd *cobra.Command, args []string) error { teamRef, _ := cmd.Flags().GetString("team") - if teamRef == "" { + linkAlertID, _ := cmd.Flags().GetString("link-alert") + if teamRef == "" && linkAlertID == "" { return fmt.Errorf("--team must not be empty") } policyID, _ := cmd.Flags().GetString("escalation-policy") @@ -50,21 +50,22 @@ func runEscalate(cmd *cobra.Command, args []string) error { if err != nil { return err } - team, err := apiClient.GetTeamByID(cmd.Context(), teamRef) - if err != nil { - return fmt.Errorf("unknown team %q: %w", teamRef, err) - } - targets := []api.NotificationTarget{{Type: api.NotificationTargetGroup, ID: team.ID}} - if policyID != "" { - targets = append(targets, api.NotificationTarget{Type: api.NotificationTargetEscalationPolicy, ID: policyID}) - } - label := incident.ID if incident.SequentialID != "" { label = incident.SequentialID } - alertID, _ := cmd.Flags().GetString("link-alert") + alertID := linkAlertID + teamName := teamRef if alertID == "" { + team, err := apiClient.GetTeamByID(cmd.Context(), teamRef) + if err != nil { + return fmt.Errorf("unknown team %q: %w", teamRef, err) + } + teamName = team.Name + targets := []api.NotificationTarget{{Type: api.NotificationTargetGroup, ID: team.ID}} + if policyID != "" { + targets = append(targets, api.NotificationTarget{Type: api.NotificationTargetEscalationPolicy, ID: policyID}) + } summary := fmt.Sprintf("Escalation for %s: %s", label, incident.Title) alertID, err = apiClient.PageAlertTargetsCLI(cmd.Context(), "", summary, note, targets) if err != nil { @@ -72,8 +73,12 @@ func runEscalate(cmd *cobra.Command, args []string) error { } } if err := apiClient.AttachAlertsToIncidentCLI(cmd.Context(), incident.ID, []string{alertID}); err != nil { - return fmt.Errorf("team %s was paged by alert %s but linking it to %s failed; retry the link without paging again with --link-alert=%s: %w", team.Name, alertID, label, alertID, err) + return fmt.Errorf("alert %s was created but linking it to %s failed; retry the link without paging again with --link-alert=%s: %w", alertID, label, alertID, err) + } + if linkAlertID != "" { + _, err = fmt.Fprintf(os.Stdout, "Linked alert %s to incident %s\n", alertID, label) + return err } - _, err = fmt.Fprintf(os.Stdout, "Paged team %s for incident %s (alert %s)\n", team.Name, label, alertID) + _, err = fmt.Fprintf(os.Stdout, "Paged team %s for incident %s (alert %s)\n", teamName, label, alertID) return err } diff --git a/internal/cmd/incidents/escalate_test.go b/internal/cmd/incidents/escalate_test.go index 3b5be98..cc5a571 100644 --- a/internal/cmd/incidents/escalate_test.go +++ b/internal/cmd/incidents/escalate_test.go @@ -135,8 +135,8 @@ func TestEscalateCmdRequiresTeam(t *testing.T) { setupTestServer(t, func(_ http.ResponseWriter, r *http.Request) { t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) }) - err := escalateCmd.ValidateRequiredFlags() - if err == nil || !strings.Contains(err.Error(), `required flag(s) "team" not set`) { + err := runEscalate(newEscalateCmd(t, map[string]string{"escalation-policy": "ep-1"}), []string{"INC-42"}) + if err == nil || !strings.Contains(err.Error(), "--team must not be empty") { t.Fatalf("error = %v", err) } } @@ -173,7 +173,7 @@ func TestRunEscalateLinkFailureNamesCreatedAlert(t *testing.T) { rec := &escalateRecorder{} setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusInternalServerError)) err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform"}), []string{"INC-42"}) - if err == nil || !strings.Contains(err.Error(), "was paged by alert alert-new") || !strings.Contains(err.Error(), "--link-alert=alert-new") { + if err == nil || !strings.Contains(err.Error(), "alert alert-new was created") || !strings.Contains(err.Error(), "--link-alert=alert-new") { t.Fatalf("error = %v", err) } } @@ -196,7 +196,7 @@ func TestRunEscalateLinkAlertRetriesWithoutPagingAgain(t *testing.T) { rec := &escalateRecorder{} setupTestServer(t, rec.handler(t, http.StatusOK, http.StatusOK)) output := captureStdout(t, func() { - if err := runEscalate(newEscalateCmd(t, map[string]string{"team": "platform", "link-alert": "alert-new"}), []string{"INC-42"}); err != nil { + if err := runEscalate(newEscalateCmd(t, map[string]string{"link-alert": "alert-new"}), []string{"INC-42"}); err != nil { t.Fatalf("runEscalate returned error: %v", err) } }) @@ -207,7 +207,7 @@ func TestRunEscalateLinkAlertRetriesWithoutPagingAgain(t *testing.T) { if len(ids) != 1 || ids[0] != "alert-new" { t.Errorf("alert_ids = %v", ids) } - if !strings.Contains(output, "alert alert-new") { + if !strings.Contains(output, "Linked alert alert-new to incident INC-42") { t.Errorf("output = %q", output) } } From f85230bd6499026a2f396f2550b6a85c900fe6d1 Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Thu, 5 Nov 2026 01:39:46 +0000 Subject: [PATCH 08/10] Verify the incident alert link after writing and retry once if it was lost Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/api/client.go | 40 ++++++++++++++- internal/api/client_alert_page_test.go | 68 ++++++++++++++++++++----- internal/cmd/incidents/escalate_test.go | 9 ++++ 3 files changed, 103 insertions(+), 14 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index fdeb78d..cde9f40 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -2780,9 +2780,47 @@ func (c *Client) listIncidentAlertIDsCLI(ctx context.Context, incidentID string) } } +// attachAlertsMaxAttempts bounds how often the link is rewritten when a +// concurrent writer drops it. +const attachAlertsMaxAttempts = 2 + // AttachAlertsToIncidentCLI links alerts to an incident. The endpoint replaces -// the incident's alert set, so the already linked alerts are sent along. +// the incident's alert set, so the already linked alerts are sent along. A +// concurrent writer can overwrite the set between our read and write, so the +// links are read back afterwards and the write is repeated once if any are missing. func (c *Client) AttachAlertsToIncidentCLI(ctx context.Context, incidentID string, alertIDs []string) error { + for attempt := 1; ; attempt++ { + if err := c.replaceIncidentAlertsCLI(ctx, incidentID, alertIDs); err != nil { + return err + } + linked, err := c.listIncidentAlertIDsCLI(ctx, incidentID) + if err != nil { + return err + } + if containsAll(linked, alertIDs) { + return nil + } + if attempt == attachAlertsMaxAttempts { + return fmt.Errorf("alerts %s are not linked to the incident after %d attempts; another update may have replaced them", strings.Join(alertIDs, ", "), attempt) + } + } +} + +func containsAll(have, want []string) bool { + set := make(map[string]bool, len(have)) + for _, id := range have { + set[id] = true + } + for _, id := range want { + if !set[id] { + return false + } + } + return true +} + +// replaceIncidentAlertsCLI reads the incident's linked alerts and writes them back with alertIDs added. +func (c *Client) replaceIncidentAlertsCLI(ctx context.Context, incidentID string, alertIDs []string) error { existing, err := c.listIncidentAlertIDsCLI(ctx, incidentID) if err != nil { return err diff --git a/internal/api/client_alert_page_test.go b/internal/api/client_alert_page_test.go index 6e39986..1e14ea6 100644 --- a/internal/api/client_alert_page_test.go +++ b/internal/api/client_alert_page_test.go @@ -139,37 +139,79 @@ func TestPageAlertTargetsCLITransportError(t *testing.T) { } } -func TestAttachAlertsToIncidentCLI(t *testing.T) { - var posted struct { - Data struct { - Attributes struct { - AlertIDs []string `json:"alert_ids"` - } `json:"attributes"` - } `json:"data"` - } +// attachServer mimics the replace-the-set endpoint. dropWrites makes that many +// updates vanish, as if a concurrent writer replaced the set right after them. +func attachServer(t *testing.T, linked []string, dropWrites int) (*httptest.Server, *[]string, *int) { + t.Helper() + posts := 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/v1/incidents/inc-1/alerts" { t.Errorf("path = %s", r.URL.Path) } if r.Method == http.MethodGet { - _, _ = w.Write([]byte(`{"data":[{"id":"alert-old"},{"id":"alert-1"}]}`)) + items := make([]string, len(linked)) + for i, id := range linked { + items[i] = `{"id":"` + id + `"}` + } + _, _ = w.Write([]byte(`{"data":[` + strings.Join(items, ",") + `]}`)) return } + var posted struct { + Data struct { + Attributes struct { + AlertIDs []string `json:"alert_ids"` + } `json:"attributes"` + } `json:"data"` + } if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { t.Errorf("failed to decode request body: %v", err) } + posts++ + if posts > dropWrites { + linked = posted.Data.Attributes.AlertIDs + } w.WriteHeader(http.StatusOK) })) - defer server.Close() + t.Cleanup(server.Close) + return server, &linked, &posts +} - client := newTestClient(t, server.URL) - if err := client.AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-1", "alert-2"}); err != nil { +func TestAttachAlertsToIncidentCLI(t *testing.T) { + server, linked, posts := attachServer(t, []string{"alert-old", "alert-1"}, 0) + if err := newTestClient(t, server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-1", "alert-2"}); err != nil { t.Fatalf("AttachAlertsToIncidentCLI returned error: %v", err) } - got := strings.Join(posted.Data.Attributes.AlertIDs, ",") + got := strings.Join(*linked, ",") if got != "alert-old,alert-1,alert-2" { t.Errorf("data.attributes.alert_ids = %s, want existing alerts kept, deduplicated, new appended", got) } + if *posts != 1 { + t.Errorf("posts = %d, want 1 when the link is confirmed", *posts) + } +} + +func TestAttachAlertsToIncidentCLIRetriesWhenLinkIsLost(t *testing.T) { + server, linked, posts := attachServer(t, []string{"alert-old"}, 1) + if err := newTestClient(t, server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-new"}); err != nil { + t.Fatalf("AttachAlertsToIncidentCLI returned error: %v", err) + } + if *posts != 2 { + t.Errorf("posts = %d, want 2 (one retry)", *posts) + } + if got := strings.Join(*linked, ","); got != "alert-old,alert-new" { + t.Errorf("linked = %s", got) + } +} + +func TestAttachAlertsToIncidentCLIFailsWhenLinkKeepsGettingLost(t *testing.T) { + server, _, posts := attachServer(t, []string{"alert-old"}, 10) + err := newTestClient(t, server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-new"}) + if err == nil || !strings.Contains(err.Error(), "alert-new are not linked") { + t.Fatalf("error = %v", err) + } + if *posts != 2 { + t.Errorf("posts = %d, want 2", *posts) + } } func TestListIncidentAlertIDsCLIPaginates(t *testing.T) { diff --git a/internal/cmd/incidents/escalate_test.go b/internal/cmd/incidents/escalate_test.go index cc5a571..dc7a4fe 100644 --- a/internal/cmd/incidents/escalate_test.go +++ b/internal/cmd/incidents/escalate_test.go @@ -60,6 +60,15 @@ func (rec *escalateRecorder) handler(t *testing.T, pageStatus, attachStatus int) case r.Method == http.MethodPost && r.URL.Path == "/v1/incidents/inc-1/alerts": _ = json.NewDecoder(r.Body).Decode(&rec.attached) w.WriteHeader(attachStatus) + if attachStatus == http.StatusOK { + // The endpoint replaces the set, so later reads see what was written. + attrs := rec.attached["data"].(map[string]interface{})["attributes"].(map[string]interface{}) + items := []string{} + for _, id := range attrs["alert_ids"].([]interface{}) { + items = append(items, `{"id":"`+id.(string)+`"}`) + } + rec.existing = "[" + strings.Join(items, ",") + "]" + } default: t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path) } From 86712c7ecf220c90081e62dcc269df111e7e22dc Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Thu, 5 Nov 2026 01:40:59 +0000 Subject: [PATCH 09/10] Return a struct from the attach test fake instead of three unnamed results Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/api/client_alert_page_test.go | 51 +++++++++++++++----------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/internal/api/client_alert_page_test.go b/internal/api/client_alert_page_test.go index 1e14ea6..82b136c 100644 --- a/internal/api/client_alert_page_test.go +++ b/internal/api/client_alert_page_test.go @@ -141,16 +141,22 @@ func TestPageAlertTargetsCLITransportError(t *testing.T) { // attachServer mimics the replace-the-set endpoint. dropWrites makes that many // updates vanish, as if a concurrent writer replaced the set right after them. -func attachServer(t *testing.T, linked []string, dropWrites int) (*httptest.Server, *[]string, *int) { +type attachFake struct { + server *httptest.Server + linked []string + posts int +} + +func newAttachFake(t *testing.T, linked []string, dropWrites int) *attachFake { t.Helper() - posts := 0 + fake := &attachFake{linked: linked} server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/v1/incidents/inc-1/alerts" { t.Errorf("path = %s", r.URL.Path) } if r.Method == http.MethodGet { - items := make([]string, len(linked)) - for i, id := range linked { + items := make([]string, len(fake.linked)) + for i, id := range fake.linked { items[i] = `{"id":"` + id + `"}` } _, _ = w.Write([]byte(`{"data":[` + strings.Join(items, ",") + `]}`)) @@ -166,51 +172,52 @@ func attachServer(t *testing.T, linked []string, dropWrites int) (*httptest.Serv if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { t.Errorf("failed to decode request body: %v", err) } - posts++ - if posts > dropWrites { - linked = posted.Data.Attributes.AlertIDs + fake.posts++ + if fake.posts > dropWrites { + fake.linked = posted.Data.Attributes.AlertIDs } w.WriteHeader(http.StatusOK) })) + fake.server = server t.Cleanup(server.Close) - return server, &linked, &posts + return fake } func TestAttachAlertsToIncidentCLI(t *testing.T) { - server, linked, posts := attachServer(t, []string{"alert-old", "alert-1"}, 0) - if err := newTestClient(t, server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-1", "alert-2"}); err != nil { + fake := newAttachFake(t, []string{"alert-old", "alert-1"}, 0) + if err := newTestClient(t, fake.server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-1", "alert-2"}); err != nil { t.Fatalf("AttachAlertsToIncidentCLI returned error: %v", err) } - got := strings.Join(*linked, ",") + got := strings.Join(fake.linked, ",") if got != "alert-old,alert-1,alert-2" { t.Errorf("data.attributes.alert_ids = %s, want existing alerts kept, deduplicated, new appended", got) } - if *posts != 1 { - t.Errorf("posts = %d, want 1 when the link is confirmed", *posts) + if fake.posts != 1 { + t.Errorf("posts = %d, want 1 when the link is confirmed", fake.posts) } } func TestAttachAlertsToIncidentCLIRetriesWhenLinkIsLost(t *testing.T) { - server, linked, posts := attachServer(t, []string{"alert-old"}, 1) - if err := newTestClient(t, server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-new"}); err != nil { + fake := newAttachFake(t, []string{"alert-old"}, 1) + if err := newTestClient(t, fake.server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-new"}); err != nil { t.Fatalf("AttachAlertsToIncidentCLI returned error: %v", err) } - if *posts != 2 { - t.Errorf("posts = %d, want 2 (one retry)", *posts) + if fake.posts != 2 { + t.Errorf("posts = %d, want 2 (one retry)", fake.posts) } - if got := strings.Join(*linked, ","); got != "alert-old,alert-new" { + if got := strings.Join(fake.linked, ","); got != "alert-old,alert-new" { t.Errorf("linked = %s", got) } } func TestAttachAlertsToIncidentCLIFailsWhenLinkKeepsGettingLost(t *testing.T) { - server, _, posts := attachServer(t, []string{"alert-old"}, 10) - err := newTestClient(t, server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-new"}) + fake := newAttachFake(t, []string{"alert-old"}, 10) + err := newTestClient(t, fake.server.URL).AttachAlertsToIncidentCLI(context.Background(), "inc-1", []string{"alert-new"}) if err == nil || !strings.Contains(err.Error(), "alert-new are not linked") { t.Fatalf("error = %v", err) } - if *posts != 2 { - t.Errorf("posts = %d, want 2", *posts) + if fake.posts != 2 { + t.Errorf("posts = %d, want 2", fake.posts) } } From 37f73ecf665724731c4ffb2f383f95d359148ffe Mon Sep 17 00:00:00 2001 From: Quentin Rousseau <879780+kwent@users.noreply.github.com> Date: Thu, 5 Nov 2026 01:42:11 +0000 Subject: [PATCH 10/10] Send data.type alerts when attaching alerts to an incident Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com> --- internal/api/client.go | 2 +- internal/api/client_alert_page_test.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/internal/api/client.go b/internal/api/client.go index cde9f40..68bc6f4 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -2835,7 +2835,7 @@ func (c *Client) replaceIncidentAlertsCLI(ctx context.Context, incidentID string } bodyBytes, err := json.Marshal(map[string]interface{}{ "data": map[string]interface{}{ - "type": "incidents", + "type": "alerts", "attributes": map[string]interface{}{"alert_ids": merged}, }, }) diff --git a/internal/api/client_alert_page_test.go b/internal/api/client_alert_page_test.go index 82b136c..2cd3f80 100644 --- a/internal/api/client_alert_page_test.go +++ b/internal/api/client_alert_page_test.go @@ -164,6 +164,7 @@ func newAttachFake(t *testing.T, linked []string, dropWrites int) *attachFake { } var posted struct { Data struct { + Type string `json:"type"` Attributes struct { AlertIDs []string `json:"alert_ids"` } `json:"attributes"` @@ -172,6 +173,9 @@ func newAttachFake(t *testing.T, linked []string, dropWrites int) *attachFake { if err := json.NewDecoder(r.Body).Decode(&posted); err != nil { t.Errorf("failed to decode request body: %v", err) } + if posted.Data.Type != "alerts" { + t.Errorf("data.type = %q, want alerts", posted.Data.Type) + } fake.posts++ if fake.posts > dropWrites { fake.linked = posted.Data.Attributes.AlertIDs