A stock PostgreSQL target without pg_wait_sampling has no wait history at all and its empty chart reads as a broken product: the collector grows a service-side pg_stat_activity sampler arm as the floor, the three wait tiers become one connect-time decision, and every wait read discloses its instrument (#3604)

[erikdarlingdata]

Wait accumulation on PostgreSQL had two paths and a hole. Aurora targets get engine-cumulative waits from aurora_stat_system_waits() (pg_wait_stats, Aurora-gated). Stock targets get a real sampled profile IF the pg_wait_sampling extension is loaded. A stock target WITHOUT the extension — most first installs, and every shop where a preload library needs a change ticket — got nothing at all except an hourly EXTENSION_MISSING skip. For the SQL Server DBA whose whole performance worldview is dm_os_wait_stats, the flagship dimension was absent on exactly the targets most likely to be their first, and an empty wait chart reads as "the product is broken".

Mechanism

PgWaitSamplingCollector now has two arms behind one definition, forked on a new connect-time fact, CollectorTargetInfo.HasPgWaitSamplingExtension (probed from pg_extension in the connected database by DarlingServerConnector, beside the Aurora probe, and carried on ConnectionProbeResult so --test-connection computes its collector count against the same shape the gate reads).

• Extension arm (fact true): the pg_wait_sampling_profile read, unchanged.
• Sampler arm (fact false): ONE multi-statement command per cycle — a pg_stat_activity snapshot, then pg_sleep(1); pg_stat_clear_snapshot(); snapshot, repeated to SamplerSnapshotsPerCycle = 30. Each snapshot is its own statement behind a clear because pg_stat_activity is cached per transaction on first access and the batch is one implicit transaction; a single statement that slept and re-read in a loop would return thirty copies of one instant. ReadAsync walks result sets by shape (four columns = snapshot, one = sleep/clear), tallies by (event_type, event, query_id), folds the window into the tally carried in through context.State, and emits the cumulative tally as rows in the extension arm's exact column shape with profile_period_ms = 1000 — so DarlingPgWaitSamplingReader's newest-minus-oldest differencing and its counter_reset arm work unchanged, and estimated_wait_ms (samples × period) is right on both arms. The tally is capped at 500 keys (the extension arm's LIMIT), least-sampled dropped.
• Which arm ran is recorded every cycle under collector_state (server_id, 'pg_wait_sampling', 'instrument') as a PgWaitInstrument token. The sampler's exclusions: its own backend, the service's other connections by application_name (pinned equal to the connector's literals), and the same three idle wait TYPES the extension arm excludes, spliced from the same set. A running backend with no wait is CPU/Running, as on the extension arm. query_id is selected only on PG14+.

Tier selection is one decision at target attach, in the issue's order: Aurora native > pg_wait_sampling present > service sampler. pg_wait_sampling.AppliesTo is now !IsAurora (was true): Aurora cannot preload the module, so the hourly EXTENSION_MISSING it recorded there was a permanent gap wearing a fixable precondition's clothes, and the sampler arm running beside pg_wait_stats would be two answers to one question on the one engine with real counters. CollectorEngineCapability.CoveredInsteadBy gains pg_wait_sampling → pg_wait_stats, the mirror of the pointer that already ran the other way, so an Aurora caller of get_pg_wait_sampling is sent to the instrument that answers instead of told to install something they cannot. The new fact is swept as FIXABLE in TargetsWithEngineKind (the existing IL-decoding guard EveryFactAPostgresGateReads_IsVariedBySweepOrFixedByKind demanded it, and passes). A new Lite test asserts over every swept PostgreSQL shape that exactly one of pg_wait_stats / pg_wait_sampling applies.

Reads disclose the instrument. get_pg_wait_sampling carries instrument (extension_sampled | service_sampled | unknown for a pre-#3604 store), instrument_recorded_at, and instrument_note — on the service tier that is PgWaitInstrument.ServiceSampledCaveat, the one copy of the floor caveat, and the empty arm states it too. get_pg_wait_stats carries instrument: engine_cumulative. Both descriptions and a new DarlingMcpInstructions section name the three tiers and the order. The Viewer's sampled-waits panel note names the arm.

Why this shape and not a third collector

The catalog's one-collector-one-table rule is load-bearing: PgSchemaGeneratorTests pins All.Count == Distinct(TargetTable).Count, and DDL generation, retention and the table census all rest on it. A PgWaitSamplerCollector writing into pg_wait_sampling would break that; its own table is a rung, and the ladder is spoken for. A target-aware definition is the established shape (cpu_utilization's Azure-vs-ring-buffer fork, pg_statement_stats' Aurora-vs-vanilla one). Provenance therefore cannot ride collection_log.collector_name — the arm records itself in the collector's own state instead, which is what the reads consult.

Cadence and detach. The collector moves from hourly to five minutes and is detached from the sequential sweep body (the third member after query_store and plan_correction, through the generic per-(server, collector) gate). Both halves of the hourly argument dissolved: it records EXTENSION_MISSING on no target now, and on the sampler arm the cadence IS the duty cycle. Awaited inline, a deliberate 30 s run would delay every other collector on a stock target by half a minute every five — the #2700 starvation with a known cause. SweepBodyDetachPolicyTests pins that nothing detached sits on the one-minute tier, which is why the cadence is five and not one.

Why 30 s at 1 Hz, and why that is a floor. SweepPressureClassifier sums every collector's single-run cost against a 60,000 ms body budget and calls it BODY_OVERRUN past that, with no detached exclusion — a four-minute sampling run would make every stock target read as saturated. Thirty seconds is the densest honest window that leaves that surface truthful; it is still 30× the resolution pg_blocking and pg_lock_stats get from one snapshot a minute. One second rather than five because each doubling of the period halves the odds of seeing a wait shorter than it, while the cost does not move: measured on the rig, one snapshot executes in 0.065 ms (1.5 ms planning) at 12 backends, from a connection the pool holds open regardless. The honest limit is in every read that serves the tier: a wait shorter than a second is seen with probability roughly its length over a second; nothing between samples or between windows (a 10% duty cycle) is seen; shares are trustworthy for steady fractions of the server's time and approximate for rare short events. The tally is persisted collector state, reloaded every cycle, so it SURVIVES a service restart; a series starts over only on cap eviction (500 keys, least-sampled dropped) or on a reversion from the extension arm, and the reader's existing counter_reset arm reports either.

Measured on the rig

timescale/timescaledb:2.28.1-pg18 as both store and stock target (no pg_wait_sampling), with an ACCESS EXCLUSIVE lock held and a blocked SELECT plus a generate_series CPU burner running, one real DarlingCollectorRunner.RunAsync cycle: wall 29,184 ms, sql 29,130 ms (29,000 of it deliberate sleep), store 13 ms, 3 series — Lock/relation seen in 30/30 snapshots with backend_count 1, CPU/Running 29/30, TimescaleDB's own workers as Extension/Extension; profile_period_ms = 1000 on every row; collector_state.instrument = service_sampled; the tally round-tripped; BuildWaitSamplingJson returned instrument: service_sampled with the floor caveat. The holder (idle in transaction, Client) and no Activity/Timeout row appeared — the shared exclusions hold on this arm.

Least privilege

Nothing beyond pg_monitor, which every PostgreSQL collector here already needs: pg_stat_activity's wait columns for other users' backends come from pg_read_all_stats, which it carries; pg_stat_clear_snapshot() and pg_sleep() are callable by any role. The first-target runbook says so where the grants are listed, and its cadence table and troubleshooting rows are updated. RequiredPgExtensions still declares the module with a narrowed meaning (the README's permissions paragraph is pinned to it, and a module dropped mid-connection on the extension arm still classifies as EXTENSION_MISSING).

What it does NOT do

• No migration, no new table, no new column. Same table, same six payload columns, same collector name.
• It does not sample Aurora. Aurora has pg_wait_stats; adding a floor beside a ceiling would be noise.
• It does not add a facet to pg_plan_capture_readiness: that collector's facets are plan-capture readiness by contract ("never a claim that a plan was captured"), and its tool deliberately reduces to no verdict. The instrument disclosure lives on the wait reads themselves and in the collector's own state; --test-connection shows the arm through the gate.
• It does not seed the tally through the delta calculator's seeding machinery; it rides StateKeys/collector_state instead (the default_trace_events re-reads the entire rollover set every cycle - validated 5.0x fix: read the current file, fall back on rollover #1962 mechanism), which is what makes it survive a service restart. The extension arm clears it every cycle it runs, so a later fallback to the sampler starts at zero rather than resuming a stale era (review catch, pinned).
• backend_count on the service tier is the WINDOW's distinct pids, not cumulative (pids recycle); the note says so.

Tests

• Lite.Tests/PgWaitSamplerArmTests.cs (13, executed here, green via the mactest pattern): exactly-one-collector over every swept PostgreSQL shape; arm follows the fact; Aurora permanent gap names pg_wait_stats; batch statement counts and clear-before-read order; window fits the body budget and command timeout; snapshot exclusions and CPU labelling; query_id by version; accumulation from a fixture result-set sequence with window-distinct backends; cumulative carry across cycles with every carried key re-emitted; empty window; tally cap; tolerant tally parse; extension arm records its instrument; StateKeys; vocabulary.
• Darling/Darling.Tests/PgWaitInstrumentDisclosureTests.cs (11, executed here): service/extension/unknown disclosure in the JSON; empty-arm sentence; the state SQL reads the collector's own row; Aurora read discloses engine_cumulative; both descriptions and the instructions name the tiers; excluded application names match the connector; connect probe SQL and ConnectionProbeResult plumbing; detach predicate.
• Darling/Darling.Tests/PgWaitSamplerLiveTests.cs (1, executed here against the rig, DARLING_TEST_PG-gated like its siblings): the run described above.
• Updated pins: SweepBodyDetachPolicyTests (third member, documented), CollectorStateContractTests (second state-declaring collector), PgWaitSamplingCollectorDefinitionTests / PgWaitExclusionParityTests contexts take the extension arm.
• Existing censuses re-executed green here: EngineCapabilityMissTests (incl. the IL gate-fact sweep), PgSchemaGeneratorTests, ReadmeDerivedCountPinTests, PgExtensionDependencyContractTests, CollectorStateContractTests, DarlingMcpPgPercentDenominatorTests — 93/93 Darling.Tests in the harness, 26/26 Lite.Tests.
• Whole solution builds -c Release -p:EnableWindowsTargeting=true with 0 warnings. WPF-only paths (the Viewer note) are compile-verified; CI is their first execution.

Closes #3604

[claude]

This new <summary> is inserted directly below the pre-existing doc comment for BuildWaitSamplingJson ("The response body, split out so the WIRE SHAPE can be asserted...", a few lines up), without moving that comment down. The net effect: the old summary is now misattached to DescribeInstrumentForEmpty (which it doesn't describe), and BuildWaitSamplingJson (below, line ~156) is left with no doc comment at all. Worth moving the pre-existing summary block down to sit directly above BuildWaitSamplingJson.

[erikdarlingdata]

Fixed in 2ee0… (second commit on this PR): DescribeInstrumentForEmpty and its own summary now sit ABOVE the pre-existing BuildWaitSamplingJson doc block, so that block is back on the member it describes. DocCommentHygieneTests.NoMemberCarriesTwoStackedSummaryBlocks executed green locally after the move (it was the CI failure on head 1).

[claude]

sb.Append(SamplerPeriodMs / 1000.0) formats the double with StringBuilder.Append(double), which is culture-sensitive (uses CultureInfo.CurrentCulture) — unlike ParseTally/SerializeTally a few dozen lines below, which explicitly use CultureInfo.InvariantCulture for exactly this reason. Right now it's harmless because 1000 / 1000.0 == 1.0 formats as "1" (no fractional part) in every culture, so pg_sleep(1) is always valid SQL. But if SamplerPeriodMs is ever tuned to a non-whole-second value (e.g. 500ms → 0.5), a Darling service host running under a culture that uses , as the decimal separator (common in much of Europe) would emit pg_sleep(0,5) — invalid syntax / wrong overload — and silently break this collector on that host. Worth using .ToString(CultureInfo.InvariantCulture) here to match the file's own convention and remove the latent trap.

[erikdarlingdata]

Fixed (third commit): (SamplerPeriodMs / 1000.0).ToString(CultureInfo.InvariantCulture), with a comment naming the trap so the next period tweak does not re-learn it. Good catch — the file already held itself to that rule in ParseTally/SerializeTally and this was the one number that slipped.

[claude]

LGTM — reviewed the pg_wait_sampling three-tier instrument change (Aurora engine_cumulative / extension_sampled / service_sampled): connect-time probe wiring, AppliesTo/CoveredInsteadBy gating, engine-capability sweep coverage, schedule-cadence justification, detach/gate wiring in DarlingWorker, the cumulative-tally sampler read and its cap/reset/parse round-trip, MCP tool and Viewer disclosure of the instrument, and Lite/Darling doc + test parity (README table, runbook, Lite.Tests mirrors). No correctness, security, or parity-drift issues found. Left two non-blocking inline notes: a misattached XML doc comment and a latent (currently inert) culture-formatting trap in the pg_sleep() literal.

[claude]

Correctness: the sampler's cumulative tally is never cleared when the target switches to the extension arm, so it goes stale and can be resumed later.

When HasPgWaitSamplingExtension is true, ReadAsync returns here without touching TallyStateKey. Since only keys present in context.PendingState get written back to collector_state (DarlingCollectorRunner only upserts PendingState, see the if (context.PendingState.Count > 0) persistence block), the old sampler_tally row is left sitting in the store untouched, potentially for a very long time (the extension can stay installed for months).

If the extension is later dropped (or the probe starts failing) and the target reconnects onto the sampler arm again, ReadSamplerAsync calls ParseTally(context.State[TallyStateKey]) and resumes accumulating from that stale, pre-extension tally rather than starting fresh:

var tally = ParseTally(context.State.TryGetValue(TallyStateKey, out var carried) ? carried : null);

In the common case the stale (small, old) tally is less than the extension arm's own cumulative counts, so DarlingPgWaitSamplingReader's (n.sample_count < o.sample_count) AS counter_reset heuristic happens to catch it and the discontinuity reads as an honest reset. But if the extension's own profile was itself recently reset (pg_wait_sampling_reset_profile() or a Postgres restart) shortly before the target falls back to the sampler arm, the resumed stale tally can be larger than the last extension-arm row, so the merge silently produces a value that looks like continued monotonic growth — no counter_reset flag — while actually splicing together two unrelated eras of data.

Given how carefully this PR reasons about staleness for the instrument token ("a target that gains or loses the extension changes arm at its next connect, and the reads must follow on the next cycle, not the next restart" / the TheExtensionArmRecordsItsInstrument test), the same care doesn't seem to have been applied to the tally. Worth clearing TallyStateKey (e.g. context.PendingState[TallyStateKey] = string.Empty) whenever the extension arm runs, so a later reversion to the sampler always starts from an empty tally instead of resuming an arbitrarily old one.

[erikdarlingdata]

Real catch — fixed in the fourth commit. The extension arm now writes PendingState[TallyStateKey] = string.Empty every cycle it runs (an empty tally is what ParseTally reads as start-over), with the reasoning you gave — including the just-reset-profile case where a stale-but-larger tally would read as monotonic growth with no reset flag — in the comment beside it. Pinned two ways in Lite.Tests/PgWaitSamplerArmTests: the extension arm's PendingState carries the empty tally, and a full sampler→extension→sampler reversion starts the fallback series at zero (27/27 executed green locally).

[claude]

Reviewed the diff (25 files, PostgreSQL wait-sampling collector's new service-side sampler arm + instrument disclosure). This is entirely shared-library/Darling C# — no T-SQL to check, and Lite never runs PostgreSQL collectors in its app (only in Lite.Tests as a cross-app parity guard), so there's no Lite/Darling UI or MCP parity gap to flag here.

Overall this is unusually well-reasoned and well-tested code (93/93 Darling.Tests + 26/26 Lite.Tests per the PR description, plus a live-rig test). I found one real correctness gap, left as an inline comment on PerformanceMonitor.Collectors/PgWaitSamplingCollector.cs:

• The sampler arm's cumulative tally (sampler_tally in collector_state) is never cleared when the target is on the extension arm. Only keys touched via context.PendingState get persisted, so if a target runs on the extension arm for a while (weeks/months) and then falls back to the sampler arm (extension dropped, preload broken, etc.), it resumes accumulating from whatever stale tally was last written before the extension took over — not from zero. In the common case this shows up as an honest counter_reset in the reader, but if the extension's own profile was itself recently reset shortly before the fallback, the stale-but-larger sampler tally can look like continued monotonic growth with no reset flag, silently splicing two unrelated eras of data together. The PR is careful about exactly this kind of staleness for the instrument token but the same treatment doesn't seem to have been applied to the tally.

Everything else I checked came back clean:

• SQL construction in SamplerSnapshotSql/SamplerBatchSql concatenates only compile-time constants (no injection surface), and the pg_sleep duration is formatted with InvariantCulture (the PR's own comment notes this was a review catch already).
• Aurora gating (AppliesTo => !IsAurora), CoveredInsteadBy mirroring, and the engine-capability sweep additions all check out against the tests that pin them.
• Detach/gating wiring in DarlingWorker (IsPgWaitSamplingCollector) is consistent with the existing query_store/plan_correction pattern, and SweepBodyDetachPolicyTests's one-minute-tier invariant still holds.
• CommandTimeoutSecondsOverride => 120 is correctly threaded through both the Lite and Darling runners.
• Docs (Darling/README.md, docs/postgres-first-target-runbook.md) match the code's actual behavior (cadence, grants, troubleshooting table).

[claude]

1. Stale cumulative tally on arm reversion (PerformanceMonitor.Collectors/PgWaitSamplingCollector.cs, ReadAsync/ReadSamplerAsync). The extension arm never touches TallyStateKey, so the sampler arm's sampler_tally state row is left untouched (potentially for months) while the extension is in use. If the target later falls back to the sampler arm, ReadSamplerAsync resumes accumulating from that stale tally instead of starting fresh. Usually this surfaces as an honest counter_reset (stale tally < extension's cumulative count), but if the extension's profile was itself recently reset just before the fallback, the stale-but-larger tally can look like continued monotonic growth with no reset flag — silently splicing two unrelated eras of data together. See inline comment for detail and a suggested fix (clear TallyStateKey whenever the extension arm runs).

Remediated in 4th commit: the extension arm clears TallyStateKey every cycle; reversion path pinned by test (sampler→extension→sampler starts at zero). Replied inline.

[claude]

Correctness: this claim about the service_sampled tier appears to be false.

The note says: "a series' counter also resets when the service restarts, which the counter_reset flag reports the same way it reports an extension reset."

But sampler_tally (the state this claim is about) is persisted to the collector_state Postgres table via StateKeys / PendingState, and reloaded from there at the start of every cycle (DarlingCollectorRunner.GetCollectorStateAsync → context.State, then SaveCollectorStateAsync after). A Darling service restart does not clear that row — the next cycle after restart reloads the same cumulative tally and keeps adding to it. So newest.sample_count will never be smaller than oldest.sample_count purely because the Darling service restarted, and counter_reset will not fire for that reason. The only things that actually reset this tally are: a key getting evicted by the 500-key cap and later reappearing, or the extension arm running a cycle (which explicitly zeroes TallyStateKey).

This isn't just a doc nit — it's a factual claim shipped in an MCP tool's response text that an operator (or an LLM client) will use to interpret a discontinuity in the data. Believing "restarting the service resets this series" is actively misleading in both directions: it invites blaming an innocent restart for a series drop that has some other cause, and it means nobody should expect a service restart to actually reset a stale/suspicious baseline.

The same incorrect claim is echoed in the PR description ("a service restart resets the service tier's series") and isn't covered by any test (no test in PgWaitInstrumentDisclosureTests.cs or PgWaitSamplerArmTests.cs exercises restart behavior). Worth either fixing the text (drop the restart claim, or correctly scope it to "restarts of this collector's key cap eviction" / whatever was actually intended) or, if a restart-reset behavior was actually intended, wiring that up in the runner.

[erikdarlingdata]

Correct, and thank you — that sentence was a leftover from an earlier draft of the design that kept the tally in process memory; once it moved to StateKeys/collector_state (so it would survive restarts, deliberately) the note did not follow. Fifth commit: the note now says the tally SURVIVES a service restart and names what does start a series over — 500-key cap eviction, or the extension→sampler reversion the previous round made clear — with counter_reset reporting either. Pinned in PgWaitInstrumentDisclosureTests.ServiceTier_IsNamedAndCarriesTheFloorCaveat (contains the survives sentence, does not contain the reset-on-restart one). PR body corrected in the same two places.

[claude]

Reviewed the diff (dev...HEAD, 25 files, +1575/-54). This is a well-designed, thoroughly-documented change — the two-arm collector design, the connect-time tier decision, cadence/detach reasoning, and Lite/Darling parity are all handled carefully, and I didn't find SQL injection, secret-handling, or missing-index-DMV issues.

One substantive finding (left inline):

1. Incorrect factual claim shipped in the MCP tool's response text. DarlingMcpPgWaitSamplingTools.cs:238 tells callers of get_pg_wait_sampling that a series' counter "resets when the service restarts, which the counter_reset flag reports." But sampler_tally is persisted to the collector_state Postgres table (via StateKeys/PendingState) and reloaded on every cycle, so a Darling service restart does not clear it — the tally keeps accumulating across restarts, and counter_reset will not fire because of one. This same incorrect claim appears in the PR description and isn't covered by any test. Since this text is read by operators and LLM clients to interpret discontinuities in the data, it should either be corrected to describe what actually resets the tally (key-cap eviction, or falling back from the extension arm) or the restart-reset behavior should actually be implemented if that was the intent.

Everything else (tier selection/gating, cadence and detach wiring, the batch SQL and result-set walking, tally accumulation/cap/parity tests, Aurora CoveredInsteadBy mirroring, Viewer/MCP disclosure, runbook updates) checked out.

Remediated in 5th commit: note corrected (tally survives restart; cap eviction / arm reversion are the resets), pinned by test; PR body corrected. Replied inline.

[claude]

LGTM — reviewed the sampler-arm collector logic (PgWaitSamplingCollector.cs), the connect-time probe (DarlingServerConnector.cs), the instrument disclosure on both MCP tools, the reader's delta/reset handling (DarlingPgWaitSamplingReader.cs), the engine-capability sweep additions (CollectorEngineCapability.cs / CollectorTargetInfo.cs), the detach-from-sequential-body wiring in DarlingWorker.cs, and the Viewer panel. No correctness, security, or Lite/Darling parity issues found — Lite never dispatches PostgreSQL collectors at all (DuckDbSchemaGenerator.StoredCollectors filters to SqlServer only), so this PostgreSQL-only change has no Lite counterpart to drift from. The SQL built in SamplerSnapshotSql/SamplerBatchSql only interpolates compile-time constants (no injection surface), the two review-fixup commits already landed (InvariantCulture on pg_sleep, tally clear on the extension arm's cycle) are present and tested, and OPTION(RECOMPILE)/T-SQL style rules don't apply since this PR touches no T-SQL.

[claude]

Reviewed the diff against CONTRIBUTING.md (PostgreSQL-store conventions: schema qualification N/A — no migration rung in this PR; naive-UTC timestamp handling checked in GetWaitInstrumentAsync's DateTime.SpecifyKind(..., Utc) on read, consistent with existing readers) and for Lite/Darling parity, correctness, and security.

Parity: PgWaitSamplingCollector lives in the shared PerformanceMonitor.Collectors project, but pg_wait_sampling is not in Lite's DuckDbSchemaGenerator.StoredCollectors roster (confirmed by grep — Lite has no pg_wait_sampling/PgWaitSampling/WaitInstrument references anywhere, and DuckDbInitializer's v61 comment states outright "Lite stores no pg_* tables"), so the new sampler arm, CommandTimeoutSecondsOverride, and StateKeys are inert there. The new CollectorTargetInfo.HasPgWaitSamplingExtension fact and the CollectorEngineCapability.TargetsWithEngineKind sweep change are exercised from Lite.Tests (census-style tests over the shared catalog), which is the existing pattern for shared-library facts — no parity gap.

Correctness spot checks that held up:

• SamplerBatchSql's statement count (30 snapshots, 29 sleep/clear pairs) matches SamplerSnapshotsPerCycle, and the culture bug for pg_sleep()'s argument was already caught and fixed in a follow-up commit on this same branch (InvariantCulture formatting).
• The tally-clear-on-extension-arm fix (also a follow-up commit on this branch) correctly prevents a stale sampler tally from being resumed as a months-old baseline after a fallback — verified by AFallbackToTheSamplerAfterTheExtensionArm_StartsFromZero.
• ReadSamplerAsync's window.TryGetValue(kv.Key, out var seen) ? seen.Pids.Count : 0 correctly short-circuits before touching seen.Pids on a carried-but-unseen key (no null-deref).
• Aurora gating (AppliesTo => !target.IsAurora), the new CoveredInsteadBy mirror pointer, the five-minute cadence, and the detached-gate wiring in DarlingWorker are all covered by both unit and live (DARLING_TEST_PG-gated) tests.
• No SQL injection surface: all string-concatenated SQL fragments (ServiceApplicationName, IgnoredTypeList, etc.) are hardcoded constants, never user/network input.

No substantive correctness, parity, or security issues found. A couple of very minor, non-blocking style notes below.

[claude]

Correction to my last comment: on closer look I don't have concrete style nits worth flagging — retracting that line. Nothing further to add.

[claude]

LGTM — reviewed the collector fork (extension vs. service-sampler arm), connect-time probe, Aurora gating, detach/cadence wiring, MCP/Viewer instrument disclosure, and Lite/Darling parity (pg_wait_sampling is confirmed absent from Lite's stored-collector roster, so the shared-code changes are inert there and no parity gap exists). No correctness, security, or parity issues found; test coverage is thorough (unit + live-Postgres-gated).