Skip to content

Alert history records whether each notification channel delivered or failed (#4750) - #4779

Merged
erikdarlingdata merged 4 commits into
devfrom
fix/4750-route-channel-outcomes
Sep 29, 2026
Merged

erikdarlingdata merged 4 commits into
devfrom
fix/4750-route-channel-outcomes

Conversation

@erikdarlingdata

@erikdarlingdata erikdarlingdata commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

Part of #4750 (the per-channel record; the failing-channel alert follows separately)

Why

With two or more channels, an alert that reached one and failed on another was recorded as delivered, with no send_error. The route record on the history row listed the channels the route resolved to, not the ones that succeeded, so a rotated Slack URL or a 5xx from one endpoint showed nowhere in the history while a sibling kept delivering.

What changes

  • AlertRouteDestinationDto gets a trailing Outcome (null by default), and a new AlertRouteOutcomes constants class holds the persisted spellings: delivered, failed, not attempted. A row written before this reads as a null outcome through TryReadRoute and TryDeserialize, the same way a row before the route record reads a null Route.
  • WebhookFanoutResult and EmailFanoutResult get a trailing ChannelOutcomes map keyed by the NotificationRouter.*Channel names. The webhook fan-out fills it in Record(channel, error) and returns it on the delivered and failed results. EmailSendCore.TrySendAsync merges it and adds Email only when an SMTP send was delivered or failed.
  • NotificationRouteDecision.ToDto(outcomes): a null map leaves the outcome null. With a map, delivered gives delivered, failed gives failed, and anything else or absent (a cooldown, a fold, a channel never reached) gives not attempted. Only channels that resolved to a destination are listed, as before.
  • No error text is stored: a webhook error can carry its endpoint URL, which is a secret. send_error and the cooldown columns do not change.
  • The map is passed where the record is attached: DarlingAlertDeliverer (engine alerts), DarlingFindingAlertSender (analysis findings) and Lite's EmailAlertService.TrySendAlertEmailAsync (which SendFindingAlertAsync goes through).
  • Lite has no routes table, so its record lists the channels the parent settings resolve to (source Default), each with its outcome. SendFindingSummaryAsync is left as it was, the same as Darling's summary path, which does not attach a route record either.
  • get_alert_history returns outcome on each destination in both editions. I grepped McpPayloadContractCensusTests and the other MCP pins in both test projects for get_alert_history and destinations; nothing pins the destination shape, so the field was added. The now-false comment in Lite/Mcp/McpAlertTools.cs (that its deliverer never writes the member) is rewritten. No tool Description string changed.
  • EmailSendCore.cs lines 316-330 (the SMTP client setup) are untouched.

Test plan

  • Darling.Tests and Lite.Tests build with 0 warnings and 0 errors.
  • AlertRouteChannelOutcomeTests (Darling, 4 tests), AlertRouteOutcomeContractTests (6) and NotificationRoutingTests: 25 passed in total. They cover a failing 500 beside a delivering channel (row is Sent, send_error null, route record delivered and failed), no HTTP text or endpoint address in the stored JSON, email joining the record, an email held by its cooldown reading not attempted, the spellings, a row without Outcome still reading, and the outcome in the MCP projection. CapturingWebhookEndpoint takes an optional status code.
  • Lite AlertRouteChannelOutcomeTests (2 tests): Generic delivered and Slack failed, both source Default, no error text; a muted alert stores no route record. 2 passed.
  • The three no-error-text assertions in the mixed-channel Darling test (HTTP 500, the endpoint URL and the loopback address all absent from the context JSON on the history row) now run and pass: AlertRouteChannelOutcomeTests 4 of 4 and AlertRouteOutcomeContractTests 6 of 6, after merging origin/dev.
  • Fail-first proof. With WebhookAlertService.cs, EmailSendCore.cs, DarlingAlertDeliverer.cs, DarlingFindingAlertSender.cs and Lite/Services/EmailAlertService.cs restored from origin/dev together (the other source files kept so the tests compile; both test projects rebuilt with 0 warnings), Darling AlertRouteChannelOutcomeTests failed 4 of 4 on the outcome assertions (expected delivered or failed, got null): AChannelThatFailsBesideOneThatDelivers_IsNamedOnTheRow_WhileTheRowStillReadsDelivered, WhenNoChannelDelivers_TheRowKeepsItsSendError_AndTheRouteRecordStoresNoErrorText, AnEmailThatWasSent_JoinsTheRecord_BesideTheWebhooksOutcomes and AChannelHeldBackByItsCooldown_IsListedAsNotAttempted. AlertRouteOutcomeContractTests (6) stayed green, as expected: it checks the destination shape, the spellings and the MCP projection, none of which sit in the restored files. Lite AlertRouteChannelOutcomeTests failed 1 of 2: AWebhookThatFailsBesideOneThatDelivers_IsNamedOnTheStoredRow (no route record on the row); the muted-alert test passed, because dev writes no route record either. The files were put back with git checkout HEAD -- <files> and the restore was never committed.
  • Also passed after the merge: Darling AlertDeliveryChannelTests, McpPayloadContractCensusTests, McpPageContractTests, NotificationRoutingTests and all seven classes in DarlingMcpAlertToolsTests.cs (there is no class of that name; the file holds DarlingMcpAlertToolsSurfaceAndSqlTests, SetMuteRuleEnabledTests, UpdateMuteRuleTests, CreateMuteRuleCoreTests, DeleteMuteRuleCoreTests, WebMuteRuleEndpointFlowTests and DarlingMcpAlertToolsLivePostgresTests), run together with the two route classes: 335 run, 0 failed, 4 skipped (the live PostgreSQL tests, which skip without a test connection string). Lite AlertDeliveryChannelTests, McpPageContractTests and AlertRouteChannelOutcomeTests: 41 run, 0 failed. Lite.Tests has no McpPayloadContractCensusTests.
  • Merged origin/dev (two commits, --configure-network keeps the web listener's tls and oidc settings, and any other key it does not ask about (#4743) #4775 and Daily rollups: days an earlier hourly repair left short are rebuilt once after the upgrade (#4716) #4763) with no conflicts; both test projects rebuilt with 0 warnings and 0 errors. Full suites, run once each with no test connection string set: Darling.Tests 17208 total, 0 failed, 1123 skipped (1121 need a live PostgreSQL test connection string; one needs symlink privileges this machine lacks and one is a Windows-specific TLS case), 1 not run (a test marked explicit); Lite.Tests 5599 total, 0 failed, 0 skipped. No code fix was needed.

CHANGELOG

SECTION: Fixed
ENTRY:

…failed (#4750)

An alert that reached one webhook and failed on another was stored as a plain
delivery: the route record listed the channels the route resolved to, and one
success stood for the whole fan-out.

The webhook fan-out now keeps each channel's outcome, the email send core adds
the email channel when an SMTP send was attempted, and the route record on the
alert-history row carries "delivered", "failed" or "not attempted" per
destination. Only that word is stored: a webhook error can carry its endpoint's
URL, so the reason stays in send_error, which does not change. Rows written
before this read as a null outcome.

Both the headless service (engine alerts and analysis findings) and Lite write
the record, and the get_alert_history tool on each edition returns it as
`outcome`. Lite has no routes, so its record names the channels the parent
settings resolve to.

Part of #4750 (the per-channel record; the failing-channel alert follows separately).
@erikdarlingdata
erikdarlingdata marked this pull request as ready for review September 29, 2026 10:15
@erikdarlingdata
erikdarlingdata merged commit 6138241 into dev Sep 29, 2026
15 of 16 checks passed
@erikdarlingdata
erikdarlingdata deleted the fix/4750-route-channel-outcomes branch September 29, 2026 10:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant