Give the generic webhook raw-JSON context tokens and the cross-channel dedup key (#2302)

[erikdarlingdata]

Closes #2302, implementing the proposal as filed — the main token plus both optional ones, since they turned out to be exactly as cheap as the issue predicted.

What lands

Three new tokens on the generic webhook channel (shared PerformanceMonitor.Notifications, so both SKUs):

• {{context_json}} — the AlertContext serialized as a raw JSON value, {} when the alert carries none. The shape is deliberately the existing AlertContextSerializer projection — byte-identical to the alert-history ContextJson — so a consumer parses ONE shape whether it reads the webhook or the history row, and the round-trip test pins that equivalence through the same serializer.
• {{incidents_json}} — just the incidents array, [] when none; the incident projection is factored so this and the full serialize cannot drift.
• {{dedup_key}} — the same key DerivePagerDutyDedupKey computes, including the stable serverId+metric fallback that existed in code but was never exposed to any consumer. The generic sender now receives the serverId the fan-out already had (same precedence as the PagerDuty call site), so tickets correlate across channels — and level/threshold alerts (High CPU, Collection Stopped, …) finally have a stable key instead of a title-matching heuristic.

The one subtlety, handled as proposed

The two *_json tokens bypass EscapeForJson via an explicit raw set in the values dictionary — they are JSON values substituted unquoted, and escaping them would turn structure back into the flattened string they exist to replace. The single-pass MatchEvaluator is untouched, so a field VALUE containing the literal text of a raw token is still not expanded (tested). A template that quotes a raw token anyway produces malformed JSON and is caught by the existing IsWellFormedJson send-path check as a config error. Alternation order is longest-first so context_json never leans on backtracking past context.

Compatibility

DefaultGenericBodyTemplate is byte-identical and a test now pins that it carries none of the automation tokens; unknown {{foo}} stays literal as before. Settings-window help text (both SKUs) and the IAlertSettings doc gained the token list, flagged as raw/unquoted.

Tests

Five new tests in GenericWebhookTests (the channel's existing suite): raw structure with a hostile Victim SQL carrying both flattening delimiters and a quote arriving byte-exact; the serializer round-trip; {}/[] for context-less alerts; all three dedup derivations (incident key, serverId fallback, serverName fallback); and the default-template pin.

Verification scope

Builds clean on macOS (Notifications, Lite.Tests, Darling service, Darling viewer — both XAML edits compile). The Windows suite can't run locally, so all scenarios were executed against the real PerformanceMonitor.Notifications build via a throwaway harness (named into the existing InternalsVisibleTo set): 19/19 pass. Not covered locally: the xUnit tests executing (CI), and a live webhook post.

One naming note for the reviewer: the issue's worked example sketched snake_case (context.incidents[].dedup_key); this ships the PascalCase persisted-ContextJson shape instead, on the argument that one canonical shape across webhook and history beats matching the sketch — flagging it since it's the only place the implementation deliberately differs from the issue text.

🤖 Generated with Claude Code

[claude]

Data exposure: {{context_json}} bypasses the T-SQL redaction every other channel enforces.

AlertContextSerializer.Serialize(context) emits AlertDetailItemDto.Body (and Remediation) unconditionally — it doesn't look at IsCodeBlock at all. But every other outbound-webhook path in this same file deliberately withholds that content from third parties and substitutes TsqlWebhookHint instead:

• Teams: WebhookAlertService.cs:345-348
• Slack: WebhookAlertService.cs:517-520
• PagerDuty custom_details: WebhookAlertService.cs:1082-1085
• This channel's own {{context}} flattening, one screen up: RenderContextForTemplate (WebhookAlertService.cs:732-736), whose doc comment literally says "Follows the Teams/Slack rule: remediation T-SQL is never inlined, it points at the email / in-app dialog."

{{context_json}} is a brand-new path that ships the full Body/Remediation payload — copy-paste remediation T-SQL, force-plan targets, etc. — raw, to whatever arbitrary URL the operator has configured for the generic webhook. That's a real change in what leaves the process for a channel whose whole reason for existing is posting to third-party automation (the PR body even lists n8n/Opsgenie/GitHub repository_dispatch as examples).

{{incidents_json}} doesn't have this problem — AlertIncidentDto never carries Body/IsCodeBlock.

Worth either redacting IsCodeBlock bodies in the context_json projection (mirroring the other channels) or, if the full-fidelity round-trip is intentional, calling this trade-off out explicitly in the settings-window help text and PR description rather than leaving it implicit.

[claude]

Comment claims a precedence match with the PagerDuty call site that doesn't actually hold on the live send path.

This does string.IsNullOrEmpty(serverId) ? serverName : serverId before deriving the key — i.e. it falls back to serverName for an empty (not just null) serverId.

The comment says this matches "the PagerDuty call site's serverId ?? serverName" — but the actual PagerDuty send path (TrySendPagerDutyAlertAsync, line 944) calls DerivePagerDutyDedupKey(serverId, metricName, context) directly, with no fallback at all. And the serverId ?? serverName this comment is presumably referring to (BuildPagerDutyPayload, line 1018) is dead on that path anyway, since dedupKey is always already non-null by the time it gets there — and even if it weren't dead, ?? only triggers on null, not on "", so it wouldn't reproduce this method's IsNullOrEmpty fallback either.

Net effect: if serverId is ever genuinely "" for a real alert, this method keys on serverName while PagerDuty keys on the raw empty string — two different dedup_keys for the same alert, which undermines the cross-channel correlation this PR is for. Every current caller happens to pass a non-empty serverId ("0" from Lite's int default, a parsed server key from Darling), so this looks unreachable today — but it's worth either fixing TrySendPagerDutyAlertAsync's call to apply the same fallback, or correcting the comment so it doesn't claim a guarantee the code doesn't provide.

[claude]

Review

Scoped to the actual diff: three new generic-webhook tokens ({{context_json}}, {{incidents_json}}, {{dedup_key}}) in the shared PerformanceMonitor.Notifications project, plus matching XAML help text in both Lite and Darling settings windows.

Parity: clean. Lite/Windows/SettingsWindow.xaml and Darling/PerformanceMonitor.Darling.Viewer/SettingsWindow.xaml got byte-identical additions, and the token logic itself lives in the shared project so both SKUs pick it up from one implementation — no drift.

Correctness of the refactor: AlertContext.cs's new ToDto(List<AlertIncident>?) extraction is behavior-preserving (identical to the inline lambda it replaces), and SerializeIncidents's "[]" fallback for a null context / null incidents is correctly exercised by the new tests. The regex alternation ordering (context_json before context) is correct and the comment explaining why it doesn't strictly need to be ordered (backtracking would find it anyway) checks out.

Left two inline comments on WebhookAlertService.cs:

1. Data exposure (the one I'd actually block on): {{context_json}} serializes the full AlertContext via AlertContextSerializer.Serialize, which includes Body/Remediation for IsCodeBlock detail items unredacted. Every other channel in this file — Teams, Slack, PagerDuty's custom_details, and this same channel's own {{context}} flattening — deliberately replaces that content with a "see email or in-app dialog" hint before it leaves the process, specifically because it's copy-paste remediation T-SQL going to a third-party URL. The new raw-JSON token skips that redaction, so an operator who wires {{context_json}} into their template now ships full remediation T-SQL to wherever the generic webhook points. {{incidents_json}} is fine — AlertIncidentDto never carries Body.
2. Minor: the doc comment on the generic channel's dedup-key derivation claims it matches "the PagerDuty call site's serverId ?? serverName" precedence, but the live PagerDuty send path doesn't actually apply that fallback (and the code it's citing is dead on that path). Currently unreachable in practice since no caller passes an empty serverId, but the comment overstates the guarantee, and if that ever changes the two channels' dedup keys would silently diverge for the same alert.

No missing-index folklore to flag, and nothing else jumped out on injection handling — the raw-token bypass of EscapeForJson is correctly scoped (both raw tokens are always well-formed JSON coming out of JsonSerializer.Serialize, and dedup_key itself is still escaped), and the well-formedness check at the send path catches template misuse.

[erikdarlingdata]

Both findings addressed in f662f0c:

1. Redaction — context_json now serializes a redacted COPY: code-block items keep their heading and IsCodeBlock flag (a consumer still learns a remediation exists and where to read it) but carry the standard "see email or in-app Alert Details" hint as their body, and the typed Remediation payload is stripped from every item. Copying rather than mutating matters — the same context instance flows on to email, which legitimately carries the T-SQL. Pinned by a test asserting the T-SQL never reaches the payload AND the original context is unmutated.

2. Doc comment — reworded to claim only what holds: the id-keyed derivation matches the live PagerDuty path; the serverName arm is this channel's own fallback for id-less callers (the settings test send), not a PagerDuty-shared guarantee.

[claude]

Save/Test-time validation won't actually catch a mis-quoted raw token.

ValidateGenericConfig (line 830) and SendTestGenericAsync (line 217) both call BuildGenericPayload with context: null (no context param on either signature), so context_json renders to the trivial "{}" and incidents_json to "[]" — neither of which contains a " character. If an operator writes "context": "{{context_json}}" (quoting the raw token, exactly the mistake called out in the doc comment above), the Save-time validation and the "Send Test" button both produce "context": "{}", which is valid JSON — so no error is surfaced.

The break only appears once a real alert with a non-empty AlertContext fires, because that's the first time the serialized JSON contains embedded " characters that clash with the manually-added surrounding quotes. And level/threshold alerts (High CPU, Collection Stopped, etc. — the ones this PR's own dedup-key fallback targets) pass context: null in production too, so this class of alert would never trip IsWellFormedJson either. An operator could have a broken generic-webhook template that "validates fine" and "test-sends fine," and only discovers it's broken when a deadlock/blocking alert with actual structured content fires — at which point it just fails silently into RecordGenericFailure/logs, not back to the settings window.

Given the header-validation code just above (HasControlChar, line ~921-924) explicitly calls out that Save/Test-time checks exist so "the operator sees it immediately," this seems like a real gap relative to that same design intent — worth exercising ValidateGenericConfig/SendTestGenericAsync with a small non-empty stand-in AlertContext (or at least one with a quote-bearing field) so the mis-quoting mistake is caught where the operator can actually see it.

[erikdarlingdata]

Fixed in cf81c64 — Save-time validation and the Test send now render the raw tokens against a quote-bearing stand-in AlertContext (one detail with a quoted field value, one incident), so the mis-quoting mistake fails at IsWellFormedJson where the operator can see it. Pinned both directions: the quoted forms of both raw tokens are rejected at Save, and the correct unquoted usage still validates null. Verified against the built assembly: the quoted template that previously validated clean now returns the well-formedness error.

[claude]

Review summary

Reviewed the diff (C#, no T-SQL changes — the shared PerformanceMonitor.Notifications project plus both apps' SettingsWindow.xaml).

Lite/Darling parity: clean. Both SettingsWindow.xaml files got byte-identical Run additions for the new automation-placeholder help text, and the actual logic lives entirely in the shared PerformanceMonitor.Notifications project consumed by both Lite/PerformanceMonitorLite.csproj and the Darling service/viewer, so there's no divergence risk there. serverId was already threaded through EmailSendCore/TrySendWebhookAlertsAsync for both SKUs pre-PR, so the new cross-channel dedup key gets a real value in production for both, not just in tests.

Redaction correctness: RedactForWebhook correctly matches the existing Teams/Slack/PagerDuty posture (code-block bodies → hint only, Remediation stripped, Fields dropped for code blocks) and is exercised by the new ContextJsonToken_RedactsRemediationTsql_LikeEveryOtherChannel test, including the important "same context instance still flows to email unredacted" check. incidents_json/the Incidents portion of context_json correctly skip redaction since AlertIncidentDto never carries the forensic DetailFields/Body/Remediation payloads that need scrubbing — consistent with the existing persisted-context shape.

Regex/token ordering: context_json correctly ordered before context in the alternation; verified this is load-bearing (leftmost-alternative-wins, not longest-match) even though the comment notes backtracking would recover the right answer anyway.

One correctness gap flagged inline: the Save-time (ValidateGenericConfig) and Test-send (SendTestGenericAsync) paths both build the validation/test payload with a null AlertContext, so the new raw context_json/incidents_json tokens render to "{}"/"[]" — values with no embedded quotes. An operator who accidentally quotes a raw token ("context": "{{context_json}}") gets a clean Save and a clean Test send, and only discovers the break when a real alert with non-empty context fires. Worse, level/threshold alerts (High CPU, Collection Stopped — the very alerts this PR's dedup fallback targets) pass context: null in production too, so that class of alert wouldn't ever surface the mismatch either. See inline comment for details.

No security issues (JSON construction goes through JsonSerializer/EscapeForJson throughout, no string concatenation of untrusted input into raw JSON), no performance concerns (webhook sends are not a hot path), and the new tests look genuinely adversarial (hostile Victim SQL with both flattening delimiters and a quote, in the "Details" round trip).

[claude]

Review: automation webhook tokens ({{context_json}}, {{incidents_json}}, {{dedup_key}})

Went through WebhookAlertService.cs, AlertContext.cs/AlertContextSerializer, IAlertSettings.cs, both SettingsWindow.xaml files, and GenericWebhookTests.cs in full.

Correctness

• BuildGenericPayload's raw-token bypass of EscapeForJson is safe: context_json/incidents_json are built via JsonSerializer.Serialize off typed DTOs, not string concatenation, so they can't produce malformed/injected JSON on their own, and IsWellFormedJson still gates the final payload as a backstop.
• Single-pass Regex.Replace correctly avoids re-expansion of one token's substituted value into another (verified the alternation ordering claim — context vs context_json — doesn't actually matter for correctness given the trailing \}\} anchor, just as the comment says).
• DerivePagerDutyDedupKey reuse for the generic channel: dedup keys will match PagerDuty's when serverId is passed (the live fan-out path), and fall back consistently otherwise. Traced TrySendGenericAlertAsync's new serverId parameter back to TrySendWebhookAlertsAsync — correctly wired, no other call sites broken (only caller is private).
• RedactForWebhook correctly strips code-block T-SQL bodies and Remediation payloads before they reach context_json, returns a copy rather than mutating the shared AlertContext that still flows to email/Teams, and the ContextJsonToken_RedactsRemediationTsql_LikeEveryOtherChannel test pins non-mutation of the original. Verified against every current producer of IsCodeBlock/Remediation items (all three live in AnalysisNotificationService.cs and always set both together), so the redaction logic's branch structure covers what's actually reachable today.
• The ValidationStandInContext() fix for the "quoted raw token validates clean against {}" trap is sound, and doesn't perturb {{context}}'s own validation (still goes through the isTest message path, not RenderContextForTemplate).
• ToDto extraction in AlertContextSerializer for SerializeIncidents is a clean refactor — same projection as Serialize, no drift risk.

Lite/Darling parity

• Both SettingsWindow.xaml files (Lite and Darling) got byte-identical additions for the new placeholder help text. No drift.
• The behavioral change lives entirely in the shared PerformanceMonitor.Notifications project; IAlertSettings only gained a doc-comment update, no new interface surface, so both apps' settings adapters pick this up automatically.

Security

• No new injection surface: raw tokens are typed-DTO JSON, not string splicing. CR/LF and header validation untouched. No new file/network/process use.

Nit (not blocking): RedactForWebhook does foreach (var detail in context.Details) without a null guard. AlertContext.Details defaults to new() and nothing in the codebase currently assigns it null, so this isn't reachable today — just noting it relies on that invariant holding.

Didn't find any correctness bugs, parity drift, or security issues worth blocking on. The test suite (redaction, round-trip, dedup fallback chain, default-template pin, validation trap) covers the risky edges well.