Chosen database states alert once, not forever — edge-trigger OFFLINE/RESTORING (#2166)

[erikdarlingdata]

What changes

The Database State alert re-fired for as long as a database stayed deviated. @gotqn's case: a soft-delete workflow parks a database OFFLINE for a month, which produced hundreds of identical alerts for one intended action.

Now OFFLINE / RESTORING / RECOVERING / STANDBY are edge-triggered — alert on the transition, stay quiet until the state changes. SUSPECT / RECOVERY_PENDING / EMERGENCY are untouched and keep re-firing on the cooldown.

The split is the whole idea: those first states are usually ones somebody chose, and repetition tells the operator nothing new. Nobody parks a database in SUSPECT, so there the repetition is the signal — there's a test pinning that an already-announced integrity state must keep firing, because the failure mode worth guarding is a real corruption quietly stopping its nagging.

Why the memory is persisted, not in-memory

This is the part that decided the design. The case is a database parked for weeks. In-memory edge state would go quiet, then re-announce every parked database on the next service restart — worse than the cooldown-repeat it replaces, and it would look like a regression to exactly the person who asked for this.

So V60 adds last_alerted_state / last_alerted_at to config.database_state_expected — the table that already holds this alert's per-database config, already keyed (server_id, database_name).

Rejected alternative: config_edge_trigger_watermarks already does restart-surviving edge memory, but it's keyed (server_id, metric_name) and metric_name feeds alert history and mute-rule matching. Per-database use would mean smuggling a compound key into a label column — fine today, a trap later.

The composition property @gotqn identified

Pinned by test: a database announced OFFLINE that turns SUSPECT fires again, at Critical. Going quiet for a parked state must not mean going blind, and the new state doesn't inherit the old one's quiet treatment.

Scope

Darling only, matching where the issue was filed. The engine change is inert for Lite by construction: with no persisted memory the adapter reports empty, current != empty is always true, and Lite behaves exactly as before. Lite's SaveDatabaseStateAlertedAsync is a documented no-op rather than a fake in-memory cache — a cache would go quiet and then re-fire on restart, which is the worst of both. Lite parity follows separately, same as #2167 → #2176.

Behavior-change warning

This alters a default. Anyone relying on the repeat-nag to track deliberate offlines will see it go quiet after the first alert — called out in the changelog in those terms.

Testing

Four engine pins: an already-announced chosen state stays silent with no cooldown involved; a first observation fires and records the state (recording is what makes the silence survive a restart); an already-announced SUSPECT still fires at Critical; and OFFLINE→SUSPECT fires again at Critical. Plus the full V60 ladder — rung identity, probe sentinel, gate arm mapping a fully-migrated store to 60 and a V59 store to 59, and the suite's ladder-tip pins moved. Alerting, Darling service, Viewer, Lite, and both test projects build clean.

Addresses #2166 (Darling half).

[claude]

This upsert breaks the "integrity states must keep re-firing" guarantee for the exact case the PR calls out as the composition property to protect.

expected_state is NOT NULL (V49), and this INSERT sets it to $3 = effectiveState — the state that was just alerted, reused for both expected_state and last_alerted_state. That's fine when a row already exists (the ON CONFLICT branch only touches last_alerted_state/last_alerted_at), but it's wrong for a brand-new row.

Concrete sequence:

1. A database with no config.database_state_expected row (never seen deviating before) goes SUSPECT. SeedDatabaseStateExpectedSql deliberately skips seeding SUSPECT/RECOVERY_PENDING/EMERGENCY on first observation ("onboarding a server mid-outage must not learn the bad state as expected... stays pending (no row)").
2. DatabaseStateDeviationsSql's expected_state IS NULL branch matches (two-sample rule), the alert fires at Critical with the "no baseline yet" message.
3. SaveDatabaseStateAlertedAsync now INSERTs a fresh row with expected_state = 'SUSPECT'.
4. Next cycle: e.expected_state is no longer NULL, and l.eff IS DISTINCT FROM e.expected_state is false (both are 'SUSPECT') — the database drops out of the deviation query entirely. No more alerts, ever, even though it's still corrupted.

This directly contradicts DatabaseState_IntegrityState_StillRepeats_EvenWhenAlreadyAnnounced (and the PR's own stated intent that SUSPECT/RECOVERY_PENDING/EMERGENCY must keep nagging on cooldown) — it's just not caught because that test stubs IAlertStateStore and never exercises this SQL against real Postgres.

Since expected_state is NOT NULL, this write path needs to either leave the pending/critical case alone (only stamp last_alerted_state/last_alerted_at, e.g. via a plain UPDATE ... WHERE EXISTS, doing nothing when no row exists yet) or explicitly avoid baselining critical states the same way the seed SQL does.

[claude]

Doc-comment/field mismatch: this V60 /// <summary> block is inserted directly after V59's existing /// <summary> block (lines ~1142-1153, "V59 — the two collector memory knobs...") with no field declaration between them. C# attaches an uninterrupted run of /// lines to whatever member follows, so both summaries merge onto V60Sql below, and V59Sql (declared right after V60Sql) ends up with no doc comment at all. Worth reordering — e.g. put V60's comment+const after V59's comment+const — so each constant keeps its own doc comment, matching how V57/V58/V59 are laid out elsewhere in this file.

[claude]

Reviewed. Solid design overall (edge-triggering only the 'chosen' states, persisting to survive restarts, keeping the integrity states on cooldown) and the engine-level test coverage for AlertEngine.cs is thorough. Two findings, left inline:

1. Correctness (high confidence) — PgAlertStateStore.SaveDatabaseStateAlertedAsync's upsert sets expected_state = effectiveState on INSERT (required since the column is NOT NULL). For a database with no existing config.database_state_expected row that goes SUSPECT/RECOVERY_PENDING/EMERGENCY — the exact "pending, no baseline yet" case SeedDatabaseStateExpectedSql deliberately leaves un-seeded — this baselines the critical state itself as "expected" on first fire. The very next cycle, DatabaseStateDeviationsSql's l.eff IS DISTINCT FROM e.expected_state check is false and the database silently drops out of the deviations list for good. That's the opposite of what DatabaseState_IntegrityState_StillRepeats_EvenWhenAlreadyAnnounced and the PR description both promise (integrity states must keep nagging on cooldown). Not caught by the new unit tests since they stub IAlertStateStore and never touch this SQL.

2. Nit — in PgMigrations.cs, the V60 doc comment is spliced directly between V59's doc comment and V59Sql's declaration, so it merges onto V60Sql and V59Sql ends up undocumented.

No Lite/Darling parity issues — the Lite no-op (LiteAlertStateStore.SaveDatabaseStateAlertedAsync) is explicitly designed and documented as inert-by-construction, matching the PR's stated scope (Darling only, Lite parity to follow separately, same pattern as #2167 → #2176). No SQL-injection or secrets concerns — all writes go through parameterized Npgsql commands.

[claude]

Correctness: this upsert can permanently silence — and falsely "resolve" — a SUSPECT/RECOVERY_PENDING/EMERGENCY database, exactly the case #2166 says must never go quiet.

Walk through a database first observed already critical (no config.database_state_expected row yet — the "pending" path):

1. SeedDatabaseStateExpectedSql (DarlingAlertReadAdapter.cs) deliberately skips baselining SUSPECT/RECOVERY_PENDING/EMERGENCY (... NOT IN ('SUSPECT','RECOVERY_PENDING','EMERGENCY')), specifically so the database stays "pending" and keeps alerting until it recovers or an operator sets an override.
2. Two critical samples in a row → DatabaseStateDeviationsSql's pending branch (e.expected_state IS NULL AND l.eff IN (...) AND p.eff IN (...)) fires the alert.
3. AlertEngine.CheckDatabaseStateAsync calls this method after delivery with effectiveState = "SUSPECT" (say). Because no row exists yet, the INSERT branch runs — and expected_state is NOT NULL, so it gets set to $3, i.e. the current critical state itself.
4. Next cycle: the row now exists with expected_state = 'SUSPECT'. The pending branch no longer matches (expected_state IS NULL is false), and the general branch's l.eff IS DISTINCT FROM e.expected_state is also false (SUSPECT == SUSPECT). The database silently drops out of current in CheckDatabaseStateAsync.
5. Because it drops out of current while still in active, the recovery block (~AlertEngine.cs L1325-1339) treats it as recovered and fires "Database State Resolved" — even though the database is still SUSPECT.
6. It will never alert again unless it later transitions to a different critical state (SeedDatabaseStateExpectedSql never overwrites the row, ON CONFLICT DO NOTHING).

So a real corruption's very first alert both permanently silences the alert and tells the operator it "resolved" — worse than the pre-PR behavior (which just kept re-firing forever), and the opposite of what the PR sets out to guarantee for integrity states.

The doc comment's justification doesn't hold up either: "the baseline column takes the current state in that case, which the seed logic would have written anyway on the next healthy observation" — but the seed logic explicitly refuses to write a baseline for these three states while they remain critical, so this INSERT does something the seed logic never would while the outage is ongoing.

None of the new tests catch this because the in-memory test harness's fake adapter/state-store are decoupled — nothing exercises the real interaction between this INSERT and DatabaseStateDeviationsSql against the same store.

Possible fix: only persist last_alerted_state for the states that actually consult it (DatabaseStateTokens.RepeatsAreNoise), or make this an UPDATE ... WHERE EXISTS so a not-yet-baselined critical database never gets an accidental baseline written.

[claude]

Correctness: recovery (below, ~L1325-1339) never clears the persisted last_alerted_state, so the same "chosen" state recurring in a later, independent episode goes silent forever.

That block clears the in-memory active set and the cooldown entry when a database returns to its expected state, but nothing calls back into _stateStore to clear the persisted last_alerted_state/last_alerted_at this line writes (PgAlertStateStore.cs L172-194).

Concretely: a database is parked OFFLINE → alerts once, last_alerted_state persisted as 'OFFLINE'. Operator brings it back ONLINE → the recovery block fires the "resolved" notice, but the persisted last_alerted_state column is untouched. Weeks later the same database is deliberately parked OFFLINE again (a second, unrelated soft-delete cycle — exactly the workflow described in the PR). DarlingAlertReadAdapter's deviation query still returns last_alerted_state = 'OFFLINE' from the first episode, so alreadyAnnounced (L1262-1264 above) is true and the second parking never alerts at all.

The "composition property" test added here (DatabaseState_ChosenState_ChangingToADifferentState_FiresAgain) only proves re-firing across a different subsequent state (OFFLINE → SUSPECT); nothing pins the same-state-after-recovery case, which is the one most likely to recur in practice for a repeated soft-delete/park workflow on the same database.

Worth clearing the persisted state on recovery (a _stateStore call in the block below), plus a test that pins: park → alert → recover → re-park with the same state → alerts again.

[claude]

Reviewed the diff. Overall the edge-trigger design and the persisted-vs-in-memory reasoning in the PR description are sound, and Lite parity (documented no-op) is correctly inert. Left two inline comments on what I believe are real correctness regressions in the persisted-memory interaction, both missed by the new unit tests because the in-memory test harness's fake adapter/state-store never exercise the real SQL round-trip together:

1. PgAlertStateStore.SaveDatabaseStateAlertedAsync — when a database is first observed already critical (SUSPECT/RECOVERY_PENDING/EMERGENCY, no baseline row yet), the upsert's INSERT branch sets expected_state to the current critical state (it's NOT NULL). On the very next cycle the database then reads as matching its own "expected" state, drops out of the deviation set, and the engine fires a false "Database State Resolved" notification — then never alerts again for that database unless it changes to a different critical state. This is the exact failure mode (SUSPECT going quiet) the PR explicitly says must never happen, and it's worse than pre-PR behavior (infinite repeat) since it now also reports a false recovery.

2. AlertEngine.CheckDatabaseStateAsync — the recovery branch clears the in-memory active/cooldown state when a database returns to its expected state, but never clears the persisted last_alerted_state. So a database parked OFFLINE, brought back online, then independently re-parked OFFLINE weeks later (the literal soft-delete-workflow scenario in the PR body) will never alert on the second parking — last_alerted_state still holds 'OFFLINE' from the first episode. The added "composition property" test only covers transitioning to a different subsequent state, not the same state recurring after a recovery.

Both are edge-memory lifecycle gaps rather than problems with the overall approach — I think both are fixable by (a) only ever persisting last_alerted_state for the states DatabaseStateTokens.RepeatsAreNoise actually consults, and/or guarding the upsert's INSERT branch so it can't invent a baseline for a not-yet-baselined critical database, and (b) clearing the persisted state in the recovery branch.

No Lite/Darling parity issues beyond what's already called out (Lite deliberately inert), and no SQL-injection or secret-handling concerns — all queries are parameterized.

[erikdarlingdata]

Both findings confirmed and fixed in 5e2a74b. Both were real, and the first one was worse than what this PR set out to fix — thank you.

The upsert. Walked the pending path and it lands exactly as described: no row → INSERT → expected_state takes the state being alerted on → the database stops deviating → the recovery block reads it as recovered and fires a false resolution on a still-corrupt database → silent forever. My doc comment's justification ("the seed logic would have written anyway") was simply wrong: SeedDatabaseStateExpectedSql explicitly refuses to baseline SUSPECT/RECOVERY_PENDING/EMERGENCY while they're critical, so this write was doing behind its back the thing it declines to do in front. Now UPDATE-only.

Worth stating why UPDATE-only loses nothing, since "skip the no-row case" sounds lossy: a row is absent only when a database was first observed in an integrity state, and RepeatsAreNoise is false for all three of those, so alreadyAnnounced never consults the memory for them — they keep alerting on the cooldown, which is the intended behavior. The parked-database case that needs the memory always has a row, because it was baselined while the database was still healthy.

The permanent memory. Also correct, and the OFFLINE → restore → OFFLINE-again sequence is the exact workflow the issue describes, so this would have bitten the reporter specifically. Added ClearDatabaseStateAlertedAsync to the interface, both stores and every stub, called on the falling edge. One deliberate choice: it clears even when suppressed, because suppression governs whether operators are told about a transition, never whether the engine keeps accurate state — leaving a stale memory behind under suppression would swallow the next real episode.

On "none of the new tests catch this." That was the most useful part of the review, because it named the structural reason rather than just the miss: the stub state store counted calls while the adapter supplied states, so nothing crossed the round trip the edge trigger actually depends on. FakeStateStore now models what the store would HOLD (save writes, clear removes), and DatabaseState_RepeatEpisode_OfTheSameState_FiresAgainAfterRecovery feeds that memory back in as LastAlertedState for the third evaluation — park, recover, re-park in the same state. It fails if either direction of the trip is missing, which the previous harness could not do. DatabaseState_AlertedStamp_IsAnUpdate_NeverAnInsert pins the SQL so the upsert cannot come back.

Verified: Alerting, Darling.Tests, Lite and Lite.Tests all build clean (the interface addition needed the Lite no-op and three stubs).

[claude]

This clear — the thing that makes the persisted last_alerted_state restart-safe against the exact bug the 3rd commit just fixed — is itself only reachable through _activeDatabaseStateAlerts (line 140), which is in-memory only and resets to empty on every process restart.

ClearDatabaseStateAlertedAsync runs for active.Where(d => !current.ContainsKey(d)) — i.e. only for databases this process instance previously observed as deviating. Walk through a restart landing between an alert and its recovery:

1. DB parks OFFLINE. Fires, persists last_alerted_state = 'OFFLINE'. active = {db}.
2. Service restarts (deploy/patch/crash) — active resets to {}. The persisted value is untouched (that's supposed to be the point).
3. Before the next eval cycle observes the database as still-deviating, it recovers to ONLINE.
4. First post-restart eval: the deviations query no longer returns this db (back to expected), so it's never in current — and active is empty, so it's never in recovered either. This line never runs. last_alerted_state stays 'OFFLINE' forever.
5. Weeks later the DB is parked OFFLINE again — the exact repeat-park workflow [FEATURE] Database State alert: per-state notify mode - fire OFFLINE/RESTORING once instead of re-firing every cooldown #2166 exists for. LastAlertedState ('OFFLINE') still equals StateDesc ('OFFLINE'), alreadyAnnounced is true, and the second parking is silently swallowed. Nobody is ever told.

This is narrower than the bug the 3rd commit fixed (needs a restart in the gap between alert and recovery, not just any recovery), but it's the same failure mode and it undercuts the PR's stated restart-survival goal. It's also a departure from how every other _active* flag in this file is documented to behave (lines 87-89: "In-memory only... the restart protection is the persisted watermarks... not these"). For those alerts, losing active on restart risks only a missed/duplicate resolution notification, because the persisted watermark that actually gates re-firing is a monotonic count/time that naturally supersedes stale values. Here the persisted value is a sticky state string whose only path to being cleared is this ephemeral set, so that invariant no longer holds for this alert.

DatabaseState_RepeatEpisode_OfTheSameState_FiresAgainAfterRecovery doesn't catch this because it drives the whole episode (alert → recover → re-park) through one AlertEngine/active instance — it never simulates a restart landing between the alert and the recovery.

A fix probably needs the clear to not depend on process-local state — e.g. have the adapter/store also identify databases with a non-null last_alerted_state whose current effective state now equals expected_state, so the memory can be cleared from what the store actually holds rather than from what this process happened to observe.

[claude]

Review

Went through the three commits (edge-trigger the chosen database states, fix the migration-doc reorder, and the author's own self-caught fixes for baseline-poisoning + falling-edge reset). Overall the design is sound and well-tested for the scenarios it targets — the UPDATE-only fix for SaveDatabaseStateAlertedAsync (never baseline a SUSPECT database via the alert-stamp write) and the falling-edge ClearDatabaseStateAlertedAsync are both real, necessary corrections.

One correctness gap remains (left as an inline comment on AlertEngine.cs:1337): the falling-edge clear that's supposed to make the new persisted last_alerted_state restart-safe is only reachable through _activeDatabaseStateAlerts, which is in-memory and resets on every process restart. If a service restart lands between an alert firing and that database's recovery, the recovery is never observed by the new process's active set, so ClearDatabaseStateAlertedAsync never runs and the persisted state goes stale forever — silently swallowing the next repeat-park episode, which is the exact workflow this PR exists to support. It's narrower than the bug the 3rd commit just fixed, but it's the same failure mode and none of the new tests exercise a restart between alert and recovery (DatabaseState_RepeatEpisode_OfTheSameState_FiresAgainAfterRecovery drives the whole episode through one engine instance).

Other notes, none blocking:

• Lite/Darling parity is handled correctly and honestly — Lite's SaveDatabaseStateAlertedAsync/ClearDatabaseStateAlertedAsync are documented no-ops rather than a fake in-memory cache, and the engine change is inert for Lite by construction (empty adapter read → every deviation reads as new, matching pre-[FEATURE] Database State alert: per-state notify mode - fire OFFLINE/RESTORING once instead of re-firing every cooldown #2166 behavior). No drift.
• SQL is all parameterized ($1/$2/$3 via Npgsql), no injection surface. Migration V60 is additive/nullable and idempotent (ADD COLUMN IF NOT EXISTS), consistent with the rest of the ladder.
• RepeatsAreNoise's XML doc only narrates OFFLINE/RESTORING in prose but the switch correctly also covers RECOVERING/STANDBY per the PR description — just a minor doc-vs-code completeness nit, not a behavior issue.
• No missing-index DMV suggestions here — not applicable to this change.

[erikdarlingdata]

Confirmed and fixed in 5be13e7. Third real one, and the diagnosis of why was the valuable part.

You're right that this alert can't follow the convention documented at lines 87-89. For the other _active* flags, losing the set on restart risks only a missed or duplicate resolution notice, because the persisted thing that gates re-firing is a monotonic count or time — a stale value is naturally superseded by the next observation. Here the persisted thing is a sticky state string whose only path to being cleared was that ephemeral set, so the invariant didn't carry over and I shouldn't have assumed it did.

Fixed by taking your suggestion literally: the clear is now derived from the store. ClearRecoveredDatabaseStateAlertsSql runs beside the existing SeedDatabaseStateExpectedSql and PruneDatabaseStateExpectedSql, on the same connection, and clears the memory for any database the store now shows back at its expected state. It can't have the gap because it asks rather than remembers, and it heals a stale memory regardless of how it got stale — restart, crash, or somebody editing the table by hand.

I kept the engine-side clear as well, and want to be explicit that this is two layers rather than accidental duplication: the engine clear is the immediate path, so a recovery inside one process doesn't wait for the next cycle's sweep, and the store sweep is what owns the invariant. If they ever disagree, the store wins by construction.

Three details that took some thought:

• It reads the same effective state the deviation query does — CASE WHEN is_in_standby THEN 'STANDBY' ELSE state_desc END — so a log-shipping secondary expected at STANDBY actually clears. Comparing against raw state_desc would have left exactly those rows stuck, which is a nasty variant of the same bug.
• It clears the (ignore) sentinel too. An operator silencing a database shouldn't leave a memory behind that outlives the silence.
• One sample at expected is enough, where the deviation rule needs two. Clearing is the safe direction — it can only cause an extra alert, never a missed one — and a flap can't exploit it, because a flap doesn't survive the two-sample deviation test to alert in the first place.

On testing: you were right that the harness test can't reach this, and I don't think any harness test can — the restart gap is defined by there being no process state, so anything driving the engine proves only the path a running process takes. So it's a gated-live test (DatabaseStateAlertMemoryLiveTests) that seeds five databases — recovered, still-deviating, deviating-differently, standby-effective-state, and (ignore) — runs the real statement against a real store with nothing in memory, and asserts a second sweep changes zero rows, since this now runs on every evaluation of every server.

Worth noting these three statements had no test coverage at all before this, so that's a small net improvement beyond the fix.

[claude]

Worth double-checking the "composition property" this PR pins (OFFLINE → SUSPECT must fire again at Critical): alreadyAnnounced correctly goes false on a state change, but the fire is still gated by CooldownElapsed(_lastDatabaseStateAlert, cooldownKey, now, alertCooldown), and cooldownKey (DatabaseStateCooldownKey) is keyed per database, not per state.

So if a database goes OFFLINE (alerts, stamps _lastDatabaseStateAlert[cooldownKey] = now) and then flips to SUSPECT within the same cooldown window, the transition-to-SUSPECT alert will be silently suppressed by the still-running cooldown from the OFFLINE alert — even though alreadyAnnounced is false for SUSPECT. That's exactly the "going quiet must not mean going blind" scenario the PR calls out as the important safety property, and none of the new tests exercise it because they all start from a fresh harness where the cooldown key has never been stamped (so CooldownElapsed trivially returns true).

This isn't introduced by this PR (the cooldown key was already shared across states pre-#2166), but the new edge-trigger feature raises the stakes: previously every deviation re-fired every cooldown regardless of state, so a same-cooldown-window SUSPECT would eventually re-announce on the next tick anyway. Now that OFFLINE goes quiet indefinitely, a fast OFFLINE→SUSPECT flip landing inside one cooldown window means the SUSPECT transition can be missed for a full cooldown period. Given SUSPECT is the integrity/safety-net case the PR explicitly designed to never go quiet, might be worth confirming this is acceptable (cooldown windows are typically much shorter than an outage) or keying the cooldown/announced-state check by state as well as database.

[claude]

Reviewed the diff (C#/Postgres, no T-SQL touched here). Overall this is solid — the design rationale in the commits/PR body is clear, and it's evident the branch already went through a few rounds of self-review (the UPSERT→UPDATE-only fix and the store-derived clear are both good catches on prior iterations).

Things I verified and didn't find issues with:

• Lite/Darling parity: intentionally Darling-only, and the "inert by construction" claim checks out — LiteAlertStateStore's new methods are no-ops, DatabaseStateInfo.LastAlertedState is never populated by LocalDataService.GetDatabaseStateDeviationsAsync (Lite's SQL doesn't select it), so RepeatsAreNoise(...) && LastAlertedState == StateDesc is always false there and behavior is unchanged. Both IAlertStateStore implementers (Lite, Darling, and every test stub) got the two new interface methods, so nothing is left unimplemented.
• V60 migration/version ladder: PgMigrations, StorageVersion, ViewerDataService.MapProbedSchemaVersion (new arg appended last with false default, correctly wired to the 43rd probe column), and all the ladder-tip test pins move together consistently.
• UPDATE-only stamp: confirmed SaveDatabaseStateAlertedAsync only ever UPDATEs (no INSERT/UPSERT path), matching the reasoning about not baselining a database first-observed in an integrity state.
• Falling-edge clear: the store-derived sweep (ClearRecoveredDatabaseStateAlertsSql) correctly reads the same effective-state CASE expression as the deviation query (standby-aware), clears the (ignore) sentinel, and runs before the deviation read each cycle so restart gaps can't leave a sticky memory — this is the right fix for the restart-window bug called out in the last commit.

One correctness edge case worth a look, left as an inline comment on AlertEngine.cs: the new alreadyAnnounced short-circuit is per-database-and-state, but the fire is still gated by the existing per-database (not per-state) cooldown key. A database that flips from an edge-suppressed state (e.g. OFFLINE) straight into an integrity state (e.g. SUSPECT) within the same cooldown window as the OFFLINE alert would have its SUSPECT transition silently swallowed by the still-running cooldown, even though alreadyAnnounced correctly evaluates to false for SUSPECT. None of the new tests catch this because they all start from a cooldown key that's never been stamped. Not something this PR introduces, but the new quiet-until-transition behavior raises the cost of that pre-existing gap for exactly the safety-critical case (integrity states) the PR is careful to protect elsewhere.

Minor/non-blocking style nit: PgAlertStateStore.cs and LiteAlertStateStore.cs both drop the blank line between the previous method's closing brace and the new method's doc comment, and leave a double blank line after ClearDatabaseStateAlertedAsync/SaveDatabaseStateAlertedAsync. Cosmetic only.

No security or missing-index concerns; the new SQL is fully parameterized and scoped by server_id.