Editing a registered server's settings in darling.json is silently ignored — only ADDING a server is warned about

[erikdarlingdata]

Found while standing up a live PostgreSQL target against a container build of dev.

What happens

WarnAboutFileOnlyServersAsync reads SELECT server_id, name FROM config_monitored_servers and compares names and ids only. If every server in darling.json is already registered, fileOnly.Count == 0 and it returns immediately. Nothing anywhere compares the file's per-server settings against the store's — grep for a property-drift comparison in StoreConfigProvider.cs finds none.

So for a server that is already registered, every setting in darling.json is dead text: trustServerCertificate, username, password, port, database, encryptMode, excludedDatabases, capturePlans, displayName. The file is read, parsed, validated, logged as Loaded configuration from /config/darling.json: 1 server(s) — and then the store's row is used instead, with no indication that the two disagree.

Why this is worse than it sounds

#2252/#2254 already warn — well — about the adjacent case:

darling.json lists N server(s) that are NOT monitored and never have been … The store is authoritative after the first seed, so adding a server to the file does not register it and a restart cannot change that

An operator who reads that learns "adding via the file does not work." The natural inference is that the file does still drive the servers it already knows about. It does not, and nothing corrects that inference.

The web.network / mcp.network blocks each log a full paragraph explaining they are file-only and restart-only and that the control plane owns the rest. The server list — far more likely to be edited than either — says nothing.

How I hit it, which is the realistic path

1. Added a PostgreSQL target. First connect failed: [pgtarget] Connect failed, retrying in 60s: SSL connection requested. No SSL enabled connection from this host is configured. Good message — names the target and the cause.
2. Enabled TLS on the target. Now: Exception while performing SSL handshake (self-signed cert, so SslMode.VerifyFull correctly refuses).
3. Applied the documented fix — set "trustServerCertificate": true in darling.json — and restarted.
4. Byte-identical error. No new information, no hint the edit was not read.

The store row explained it:

name     | trust_server_certificate |         created_at         |        modified_at
pgtarget | f                        | 2026-08-22 22:27:15.069708 | 2026-08-22 22:27:15.069708

created_at == modified_at, still f. The edit never left the file.

That is a bad loop to be stuck in: a connection failure is exactly the kind of problem an operator fixes by editing config and restarting, and this is the one class of edit that produces an unchanged error with no explanation.

Worth deciding, not assumed

I am not proposing that the file start overriding the store — store-authoritative is a deliberate design and #2254 is explicit about it. The gap is that the disagreement is invisible. The minimum fix is to extend the existing warning to cover servers present in both, comparing the settings that actually affect behaviour and naming the ones that differ, in the same voice as the existing message: what the file says, what the store says, and that the store wins and how to change it (Viewer's Add/Edit Server, or MCP add_servers).

Two things to settle first:

• Which fields count. Comparing everything invites noise from cosmetic normalization — encrypt_mode is already normalized on seed (StoreConfigProvider.cs:350 mirrors the normalizer precisely so a casing choice cannot fail the seed), so a naive comparison would warn on a difference that does not exist. The comparison has to run against the same normalizer.
• Passwords must not be compared or printed. The file may carry a file:/env: reference while the store carries a DPAPI blob; those are not comparable and the answer is almost certainly to exclude the credential from the drift check entirely rather than to try.

A --test-connection caveat probably belongs here too: per the #2252 message it already reads darling.json and reports PASS for servers that collect nothing. The same is true for settings — it would test the file's trustServerCertificate, not the store's, so it can report PASS for a configuration the service will never use.

Related: #2252, #2254, #2258.

[erikdarlingdata]

Fix is up in #2556 (targets dev, not merged). The report holds — the mechanism, the reproduction and the "the #2254 warning makes this worse" reading are all confirmed against the source. Three corrections to the framing, one of which changes what the message can say.

1. capturePlans is not a per-server setting. MonitoredServer has no such property; capturePlans is a top-level service setting seeded into config_service. config_monitored_servers.capture_plans does exist, but the seed writes it NULL unconditionally, so there is no file value to disagree with it. Every other field in the list is a real per-server setting and every one of them was dead text.

2. add_servers cannot be part of the remedy. This is the one that changes the message. add_servers refuses an already-registered server: PartitionDuplicates skips it as status:"duplicate" before it validates anything. So "the Viewer's Add/Edit Server dialog, or MCP add_servers" would have sent an operator to a tool that silently does nothing — the exact failure the warning exists to end. There is no MCP edit verb at all; remove_server + add_servers is the only headless path and it is a heavier hammer than a startup warning should recommend. The line names the Viewer's Manage Servers window and rules add_servers out explicitly, echoing the Viewer's own duplicate message ("Edit it from Manage Servers instead").

3. StoreConfigProvider.cs:350 is the plan-XML normalizer, not the encrypt_mode one. That line is NormalizePlanXmlCompression, mirroring V62's case-sensitive CHECK on plan_xml_compression. encrypt_mode has no CHECK constraint and the seed writes it raw (line 512). So the hazard is real but it lives at the connect path, not the seed: MonitoredServerConnection folds the mode with Trim().ToUpperInvariant() and fails closed to Mandatory, which is why "strict" vs "Strict" — and a typo vs "Mandatory" — build the same connection. The comparison folds through that switch rather than through a seed-side normalizer.

The audit you asked for turned up five more folds of the same kind, not just encryptMode:

• a blank database is the engine's implicit default, so a NULL column against an explicit "master" (or "postgres") is one connection;
• engine is stored raw by design (V68), so "aurora" and "postgres" routinely sit on opposite sides of one identity — folded through TargetEngine;
• PostgreSQL port 0 means the driver default, i.e. 5432;
• excludedDatabases is spliced as a NOT IN set by DatabaseExclusionFilter, so order, repetition, case and whitespace change nothing;
• monthlyCostUsd is a decimal, which carries its scale, so 1200 and 1200.00 render differently and are one figure.

Three fields are also gated rather than compared, because they are inert on the target they sit on: readOnlyIntent / multiSubnetFailover are SqlClient concepts the Npgsql builder never sees, port is not a SQL Server concept (it rides in the host as host,1433), and username only reaches the connection string under SQL auth. The gates read the store's engine and auth, since the store is what the service connects with; a disagreement about the engine itself gets its own line.

On the credential: excluded entirely, and structurally rather than by intention. encrypted_password is not in the SELECT list, so there is no blob in memory for a later edit to leak. And it must not be compared even if it were read — a file entry carrying an env:/file: reference or a dev plaintext password against a store row carrying a DPAPI blob is the supported shape that BuildServerFromRow's read-time backfill exists to serve, so comparing them would report a working configuration as drift on every start. Proven by reverting: adding the comparison puts an env: reference and a DPAPI blob straight into the rendered warning.

On the --test-connection caveat: yes, but conditionally. It is appended only when a connection-relevant field drifted. The verb probes the file's connection settings, so it can report PASS for a connection the service will never make — which is your reproduction exactly. But it does not exercise the display name, the exclusion list, the cost figure or the delivery override, so appending it there would be claiming more than the warning can support.

Also picked up your point about the server list saying nothing: darling.sample.json's servers block now carries the seed-once paragraph the web.network / mcp.network blocks have always had, including which two things in that block are still read every start (the SQL-auth secret, and the whole file when the store is unreachable).

The store still wins. Nothing about store-authority changed.

[erikdarlingdata]

Fixed in #2556 (merged to dev), and verified live — the one thing that PR listed as unverified.

I reproduced the original scenario on a running service against a real seeded store: took a registered PostgreSQL server and edited four of its settings in darling.json so the file disagreed with the registry on database, username, trustServerCertificate and port. On restart, the service now says so:

darling.json disagrees with the registry about 1 monitored server(s), and the registry is what the service uses: pgtarget: database (file=wrongdb, store=appdb), username (file=someoneelse, store=darlingmon), trustServerCertificate (file=false, store=true), port (file=5555, store=5432). The store is authoritative after the first seed, so editing a registered server's settings in the file changes nothing and a restart cannot change that — edit them in the Viewer's Manage Servers window (the MCP add_servers tool cannot: an already-registered server is skipped as a duplicate). Note --test-connection reads darling.json, so it probes the FILE's settings and can report PASS for a connection the service will never make.

Every drifted field named, both values shown, the remedy correct, and the --test-connection caveat present. That exercises the live SELECT shape, the MonitoredServer hydration from the reader, and the log line as the service's own logger renders it — none of which CI's fixtures cover.

Three corrections to this issue, all mine:

1. capturePlans is not a per-server setting. MonitoredServer has no such property — it is top-level, and the store's per-server column seeds NULL. I listed it as silently-ignored; it was never file-driven per server.
2. add_servers cannot be the remedy, and recommending it would have reproduced this very bug. PartitionDuplicates skips an already-monitored server as duplicate before validating anything, so an operator following that advice would have been sent somewhere that silently does nothing. There is no MCP edit verb at all. The shipped message names the Viewer's Manage Servers window and rules add_servers out explicitly.
3. StoreConfigProvider.cs:350 is NormalizePlanXmlCompression, not the encrypt-mode normalizer I claimed. encrypt_mode has no CHECK constraint and the seed writes it raw. The normalization hazard I described is real, but it lives at the connect path — so the comparison folds through MonitoredServerConnection's own fail-closed switch rather than a seed-side normalizer.

The comparison turned out to need more care than I credited: four fields are engine-asymmetric — Strict ≡ Mandatory on PostgreSQL only, blank database folds to an engine-specific default, PG port 0→5432, and database/excludedDatabases are case-sensitive on PostgreSQL but not SQL Server. Inert fields are not compared at all, gated on the store's engine and auth mode.

Credentials are excluded structurally rather than by a rule someone could later relax: encrypted_password is simply not in the SELECT list, so no blob exists in memory to leak.

One review finding is worth recording because the obvious fix was wrong: filtering disabled servers out of the store read would have caused #2158, since that same read feeds ServersOnlyInFile — a paused server would then be reported as never-monitored. Disabled servers get their own Information line instead.