[BUG] #2349's file-growth knobs have no Settings window or MCP surface, so an alert that ships OFF cannot be enabled from any supported surface #2391

Activity

1. gotqn

   added

   bugSomething isn't workingSomething isn't working

   on Aug 21, 2026
2. erikdarlingdata commented on Aug 21, 2026

   

   erikdarlingdata

   on Aug 21, 2026

   Owner

   More actions

   Confirmed on every point, and this one is mine — I built #2349 and shipped the store plane without the two surfaces that make it reachable.

   Verified rather than taken on trust:

   • MCP: zero occurrences of file_growth anywhere under Mcp/. No group in get_alert_settings, nothing accepted by update_alert_settings.
   • Darling Viewer: the only FileGrowth references are the schema-version probe (hasFileGrowthAlert, for the V79 banner) and an unrelated FileGrowthTargets remediation. No Settings control, and the "Will alert when: …" summary line is untouched.
   • Lite: same shape — AppAlertEngineSettings and the DuckDB read are wired, the Settings window is not.

   And your read on why it looks unwired rather than deliberate is exactly right. DarlingAlertSettings already clamps all four the way their siblings are clamped:

   public bool FileGrowthEnabled          => _config.Alerts.FileGrowthEnabled;
   public int  FileGrowthRiseMb           => Math.Max(0, _config.Alerts.FileGrowthRiseMb);
   public int  FileGrowthVolumePercent    => Math.Clamp(_config.Alerts.FileGrowthVolumePercent, 0, 100);
   public int  FileGrowthLookbackMinutes  => Math.Clamp(_config.Alerts.FileGrowthLookbackMinutes, 5, 1440);

   That is the tell: the half that is hard was done, and the half that makes it usable was not.

   The #2107 comparison is the fair one to hold this to — "six new knobs ride the store control plane, the Viewer's Settings window, and get_alert_settings/update_alert_settings". #2349 shipped one of the three. Worse than a knob that is merely awkward to reach: this alert ships OFF by design, so the enable checkbox is not a convenience, it is the only door in. Shipping it without one meant shipping a feature you asked for in a state where you cannot turn it on without writing SQL at the monitoring store. You are right not to want that in a runbook, and I would not want it in mine.

   Fixing it in that order, MCP first, because that is the one that gets the UPDATE out of your runbook soonest — a file_growth group on both tools, clamped on read like its siblings. Then the Viewer's Settings window on both SKUs, including the "Will alert when: …" summary, which is its own small bug: a summary that silently omits an enabled alert is worse than no summary.

   Not going to pretend this is anything other than an unfinished feature. Thanks for the precise report — the file-by-file inventory meant I could confirm it in one pass instead of hunting.
3. erikdarlingdata

   added a commit that references this issue on Aug 21, 2026

   Expose the file-growth alert knobs in both Settings windows

   ...

   b30622a
4. erikdarlingdata commented on Aug 21, 2026

   

   erikdarlingdata

   on Aug 21, 2026

   Owner

   More actions

   Fixed on dev in #2392 — thanks for the report, @gotqn, and for holding it against the #2107 precedent. That was the right standard: #2349 shipped the store plane and stopped, which for an alert that ships OFF meant a hand-written UPDATE against config_alert_settings was the only supported way to turn on a feature someone had asked for.

   All three surfaces now carry the four knobs:

   • MCP — get_alert_settings reports a file_growth group; update_alert_settings accepts enabled, rise_mb, volume_percent, lookback_minutes.
   • Darling Viewer Settings — the control row plus seed/save/parse/gate/restore-defaults, and the Will alert when: summary, which previously could omit an alert that was enabled and able to fire.
   • Lite Settings — the same row against its App.Alert* statics, and the four writes into settings.json.

   The service side needed nothing: StoreConfigProvider has selected and read all four columns since #2349. The gap was only ever the surfaces a user can reach.

   Two guards went in with it, both aimed at the class of mistake rather than this instance. One derives the column count from AlertSettingsSelectSql and the ordinals from the reader and asserts they agree, because every field on that record is positional — a column inserted anywhere but the end silently re-maps everything after it and usually still compiles. The other pins that Lite's SaveAlertSettings writes every static it assigns; that method copies controls into statics and then statics into JSON, and skipping the second line is invisible in manual testing because the setting takes effect immediately and only reverts on the next launch.

   Write bounds match the engine's clamps exactly, pinned by a test that reads both files, so the tools can't accept a value the engine quietly rewrites on read. Zero stays valid on rise_mb and volume_percent because #2349 uses zero to disable a gate — a floor of 1 would have removed the rise-only and level-only configurations.

   Two adjacent gaps this turned up are filed separately rather than folded in: #2393 (three settings keys Lite loads but exposes nowhere) and #2394 (Lite's MCP reports 4 alert groups to Darling's 48 and cannot write any).

   Closing against dev; this reaches a build at the next release.
5. erikdarlingdata

   closed this as completedon Aug 21, 2026
6. MisterZeus

   added a commit that references this issue on Aug 24, 2026

   Put the file-growth knobs on the MCP alert tools

   ...

   699839b