Skip to content

[IR-6586] feat(02-03): warn when resolving an incident from the CLI - #83

Merged
kwent merged 11 commits into
masterfrom
soil/ir-6586-cb8ca4
Nov 11, 2026
Merged

kwent merged 11 commits into
masterfrom
soil/ir-6586-cb8ca4

Conversation

@kwent

@kwent kwent commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

Why

Resolving an incident from the CLI does not touch the public status page entries linked to it, and nothing tells you they are still open. Subscribers keep seeing an active incident. See #52. The status page commands themselves already shipped in #57 (v0.6.1) as status-pages list and status-pages events list/create/update/resolve, so this PR does not add or rename any command.

What changed

A new best-effort helper package, internal/statuspagewarn, prints a warning to stderr when a command leaves an incident resolved or closed and that incident still has open status page events. Each open event is listed with its status page ID, its status and the exact command to close it.

Where it fires (one call per command):

  • incidents update when the returned incident status is resolved or closed
  • incidents create when the created incident is resolved or closed
  • alerts resolve --resolve-incidents, for each related incident that is resolved or closed

Details:

  • Events whose status is resolved or completed are not listed. No events, or an incident that is not left resolved, means no output and no extra request.
  • Incident events get rootly status-pages events resolve <id>. Open maintenance events (scheduled, in_progress) get --status=completed added, since maintenance is closed with completed.
  • Best-effort: the whole check shares a 10 second timeout. If the lookup fails or times out, the command still succeeds with the same exit code and the same stdout. The error is only shown as a debug: line when --debug is set.
  • All event pages are inspected, following total_pages.
  • stdout is unchanged, so --format json and --format yaml output stays clean.
  • Nothing is auto-resolved. Resolving a status page entry is a customer-visible write that can notify subscribers, so the CLI only points at the command to run. No flag was added and no existing command or flag was renamed. An opt-in flag to resolve linked events could be a follow-up.
  • Help text for incidents update and alerts resolve, and the README, mention the warning.

Example stderr after rootly incidents update INC-123 --status=resolved:

Warning: incident INC-123 is resolved but 2 status page event(s) are still open:
  status page 3f1c..., status investigating: rootly status-pages events resolve 9a7b... --message="<public resolution message>"
  status page 8d20..., status in_progress: rootly status-pages events resolve c41e... --status=completed --message="<public resolution message>"
  --message is the public text shown on the status page; resolving may notify subscribers.

How to test

go test ./internal/statuspagewarn/... ./internal/cmd/incidents/... ./internal/cmd/alerts/...
make coverage && make coverage-check

The unit tests use httptest servers to cover open and mixed events, pagination, maintenance statuses, silent cases, lookup failures, a stalled endpoint, and the alerts resolve --resolve-incidents path.

@kwent
kwent requested a review from a team November 4, 2026 21:08
@kwent kwent added the soil:override Opened by a Soil automation from Soil:override label Nov 4, 2026
@soil-cloud

soil-cloud Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Pushed 57d34a8: named the results of the newClient test helper to clear a gocritic unnamedResult finding. golangci-lint v2 now reports 0 issues, and total coverage is 80.45% (floor 80%). As noted for the council, the idioms, standards and criteria seats did not run; the general and deep seats did.

@soil-cloud

soil-cloud Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Soil is watching this PR. It acts on CodeFlow's review of the latest commit, review comments and change requests, red CI (after checking master and re-running once for flakes) and merge conflicts: one commit per fix, with a reply in each thread it handles. It skips comments containing "(aside)".
The PR's owner can comment stop (or stop: <reason>) to have Soil close this PR.

  • Disable Soil's automatic review, CI and merge-conflict monitoring

@greptile-apps

greptile-apps Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge; no new actionable issues remain.

Summary

Adds stderr warnings when resolved incidents still have open status-page events.

  • Incident resolution now warns when public status-page events remain open.

Reviews (4) · Last reviewed commit: "statuspagewarn: release the stalled hand..." · Reviewed by Greptile

@soil-cloud

soil-cloud Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

@greptileai

1 similar comment
@soil-cloud

soil-cloud Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

@greptileai

@soil-cloud

soil-cloud Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

@greptileai

@soil-cloud

soil-cloud Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Soil council

Soil's council reviewed this change across 3 commits (d2081ce to 7ee88cf): 7 findings. 4 fixed before posting (4 re-checked gone at a later commit), 2 rebutted, 0 the lane could not fix.

Rebutted, with reasons (2)

  • internal/statuspagewarn/statuspagewarn.go:53: A mid-pagination lookup failure prints a partial open-event count as if it were complete. not worth changing: On a mid-pagination failure the warning lists only the open events it did see, which is still accurate and useful. The check is best-effort by decision: a failed lookup is reported as a debug note at most, so the command output stays quiet and unchanged. Counting from earlier pa… (the author's reason)
  • internal/statuspagewarn/statuspagewarn.go:85: One 10s deadline is shared across the alert lookup and every related incident, so later incidents are silently skipped. intentional: One shared deadline across the alert fetch, all pages and all related incidents is a deliberate trade-off, and was requested in review so the optional check can never delay the command by more than 10 seconds in total. Per-incident timeouts would multiply that bound. A late time… (the author's reason)

Open suggestions (1), not blocking:

  • internal/cmd/incidents/update.go:169: Warning fires on any update that ends with a resolved incident, not only when --status is set
Fixed before posting (4)
  • internal/statuspagewarn/statuspagewarn.go:34: Open events beyond the first page are silently missed, fixed in 05d67b6, re-checked gone at 05d67b6
  • internal/statuspagewarn/statuspagewarn.go:50: The suggested resolve command fails required-flag validation, fixed in f343808, re-checked gone at f343808
  • internal/statuspagewarn/statuspagewarn.go:46: Pagination stops early when only total_pages indicates more results, fixed in d2081ce, re-checked gone at d2081ce
  • internal/statuspagewarn/statuspagewarn.go:40: Ignored pagination duplicates open events in the warning, fixed in d2081ce, re-checked gone at d2081ce

kwent and others added 8 commits November 5, 2026 01:35
…ent is resolved

Adds internal/statuspagewarn and calls it from incidents update/create and
alerts resolve --resolve-incidents. Best-effort, stderr only, warn only.

Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
…e command

Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
…eted for maintenance

Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
…oses

Co-authored-by: soil-cloud[bot] <344511034+soil-cloud[bot]@users.noreply.github.com>
@kwent
kwent force-pushed the soil/ir-6586-cb8ca4 branch from 4144108 to 7ee88cf Compare November 5, 2026 01:36
@soil-cloud

soil-cloud Bot commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Rebased onto the latest master. Upstream only changed README.md (docs for incident association flags), with no conflicts and no code overlap. Vet, tests and lint pass, and total coverage is 80.5%.

@kwent
kwent merged commit 3ae3c06 into master Nov 11, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:medium soil:override Opened by a Soil automation from Soil:override

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant