Darling MCP declares its peer stores instead of answering "unknown server" (#2339)

[erikdarlingdata]

Closes #2339 — tier 1 only (declared peers / disclosure). No federation, no cross-store connectivity, and none is built here.

The problem

The fleet is now split across several Darling boxes, one store each: the use1 box monitors 42 SQL Server primaries, the use2 box monitors their readable replicas, a third box is heading for PostgreSQL. Each box's MCP server answers over its store only, and nothing in the instructions, list_servers, or any error named the split — so an agent connected to the use2 endpoint asking about prod-sql-use1-beta-01 got Could not resolve server, which is indistinguishable from "nobody monitors that server." With the fleet split by design, that is now the normal case rather than an edge.

What this adds

An optional peers block in darling.json — thisStoreCovers, plus per-peer name / covers / optional matches name-substrings — disclosed at the places an agent forms its picture of the fleet:

1. The MCP instructions gain a Fleet Coverage section, placed between the read-only preamble and the tool census so an agent learns which store it is talking to before it starts planning against the tool list.
2. list_servers gains this_store_covers, a peer_fleets array, and a peer_note — including on the empty-registry answer, which is prose rather than JSON and would otherwise be the one path where the declaration silently vanished (a store with nothing registered is a fresh or just-restarted box, the worst place to drop it).
3. The server-resolution miss appends the disclosure to the existing available-servers listing.

The miss now reads (verbatim from the harness):

Could not resolve server. Available servers:
prod-sql-use1-beta-01

'prod-sql-use2-beta-01' is not monitored HERE, and it matches the declared coverage of peer
store prod-sql-use2-monitor-01 — the readable replicas of those same 42 primaries, from
us-east-2. That is a SEPARATE Darling store with its own MCP endpoint; this server cannot
read it, so point the client at that endpoint (or tell your operator which store answers for
this server) rather than concluding the server is unmonitored. This store covers: the 42
us-east-1 SQL Server primaries.

It is appended, not substituted: Could not resolve server. is the prefix callers (and several tests) key off, and the local listing is still the right answer to the commonest miss, a typo.

Decisions worth reviewing

Disclosure only, and it says so everywhere. No address and no credential in the block, and nothing behind it — the service never contacts a peer, cannot read one's data, and cannot tell whether one is running. Every message states that, because an agent told a sibling exists will otherwise try to route a query at it.

A peers block that fails validation is refused whole. All of this text is sent verbatim to every connected MCP client, so PeersConfig.Validate rejects password= / connectionString / Integrated Security= etc. in any peer string, and a nameless peer (an agent told "some other store has it", with no name to point a human at, is no better off than before). The guard runs inside DarlingPeerDirectory.Publish, not only in config validation — see the review round below for why that distinction turned out to be load-bearing. A refusal publishes Snapshot.Empty: the whole block, not the valid subset, because an unfinished block asserting coverage that may be wrong is worse than none. A peer with a name and no covers is fine and renders as just the name.

matches is plain case-insensitive substrings — no globbing, no regex. It exists to answer "which region/role prefix is this name?", and a pattern language is a config surface with its own failure modes. Blank entries are dropped: an empty substring matches every name and would make one peer claim the whole fleet. A peer that declares none is still disclosed everywhere, it just cannot be singled out on a miss — and the miss message says that rather than implying the server is unmonitored.

An empty peer_fleets carries its own note. It means either this is the only Darling store or the operator never declared the siblings, and the service cannot tell those apart — so peer_note says exactly that instead of letting an empty array read as "you are looking at the whole fleet."

The snapshot is ambient, not injected. ~90 tool methods resolve a server name through DarlingServerResolver taking only an NpgsqlDataSource; threading a parameter through all of them plus the web dashboard's read dispatch would touch every one to deliver a constant. It is an immutable record behind a volatile field, published from both the worker and the MCP host (either may reach its config first; they read the same file so they cannot disagree). Every pure function also has an explicit-snapshot overload, which is what the tests use.

Nothing declared = nothing changes, with one named exception. Snapshot.Empty is the default, so the three prose surfaces are byte-for-byte what they were: the instructions (Build(Empty) pinned by reference equality with Text), the resolution miss (pinned by exact string equality), and list_servers' empty-registry sentence.

The exception, raised in review and now called out in the CHANGELOG, the test class doc and the README rather than folded into an "unchanged" claim: list_servers' JSON envelope carries this_store_covers / peer_fleets / peer_note on every response, declared or not — so a client comparing that tool's exact shape sees three new keys on upgrade even if it never writes a peers block. That is the design, not an oversight: a note that appeared only once peers were declared would say nothing in precisely the case that produces the wrong conclusion.

Lite gets no twin — it has no peers concept and no central store — and nothing in PerformanceMonitor.Collectors / PerformanceMonitor.Common was touched.

Review round

Three findings from claude[bot], all reproduced before fixing, all answered on their threads.

1. The credential guard never covered the path that broadcasts. PeersConfig.Validate ran only from DarlingConfig.Validate, which only the worker calls — and the worker's abort is a return from its own hosted service, not a process exit (verified: no StopApplication or Environment.Exit anywhere in Program.cs). The MCP host loads its own copy of darling.json and deliberately never validates it, because its fail-closed checks are host-local by design. So a credential pasted into a peer description stopped collection and then went out to every connected client — and with the aborted worker never reaching its own publish, the host's unvalidated publish became the only value in the ambient snapshot. Fixed at the category rather than the call site: Publish now validates and refuses, returning a PublishResult record struct carrying the problems so a caller cannot take the snapshot and drop the reason. The ambient snapshot can only be written through Publish, so a future third publish site cannot reintroduce the hole.
2. An explicit "stores": null skipped the thisStoreCovers guard. System.Text.Json assigns null over the property initializer (an omitted key leaves the default), and the self-text check sat after the per-peer loop behind an early return. Reproduced — Validate() problems: 0 with the credential reaching the rendered disclosure — then fixed by ordering rather than a second check: an unconditional guard cannot be bypassed by a shape nobody enumerated. My own tests missed it because they all built PeersConfig in C#, where the initializer guarantees a non-null list.
3. The "byte-for-byte unchanged" claim overstated the list_servers case — corrected above and in three files; documentation only, no behavior change, and the pin that exposed the discrepancy was left exactly as it was.

Also folded in: the multi-claimant resolution miss now agrees in number. Two peers can legitimately both claim a name through overlapping matches, and the follow-on sentence said "That is a SEPARATE Darling store" about a list of two.

Verification

PerformanceMonitor.Darling.Service, Darling.Tests and Lite.Tests all build clean (EnableWindowsTargeting, 0 warnings). The Windows-only suites cannot run on macOS, so CI is the arbiter for the 23 new xUnit facts in Darling/Darling.Tests/DarlingPeerDisclosureTests.cs.

To not defer the logic to CI, a throwaway net10.0 console harness ran 114 assertions against the real build: config parse/validate (including the shipped sample), normalization and matching, the instructions section and its placement, the rendered list_servers JSON in both the declared and empty cases, the empty-registry prose, all five resolution-miss shapes, the ambient publish/refuse/reset, and both code findings — including that leaked text reaches neither the rendered instructions nor a resolution miss, and that the valid peer in a refused block is not disclosed either. Negative control: neutering ResolutionMissDisclosure turns 9 of them red, so they test the fix rather than passing by accident.

Two specific pins that could have bitten silently:

• The cross-app census pin (Lite.Tests/CrossAppMcpToolInventoryPinTests.DarlingInstructionsCensus_MatchesTheScannedInventory) parses the tool-census sentence out of DarlingMcpInstructions.cs. Splitting the const into Preamble/Body leaves that sentence intact; the harness re-runs the pin's own regex against the shipped string and checks its arithmetic.
• darling.sample.json is the real fixture for DarlingNetworkConfigEditor's comment-preserving text surgery, and adding a peers block moved which root member is last. The harness re-ran every upsert/remove against the edited sample (all three network parents, plus the new one) and confirmed each result still parses with the peers block intact.

Not verified locally: the xUnit suites themselves, and no live-store or on-box behaviour — the disclosure is pure over config, so there is no collector or store path to exercise.

One thing deliberately not changed: that the worker's Validate() abort does not stop the process is pre-existing for every config problem (servers must contain at least one entry behaves the same way), so it is a separate question from this PR.

🤖 Generated with Claude Code

[claude]

The credential-shaped-text refusal doesn't actually gate this path — the one that broadcasts to clients.

PeersConfig.Validate is only ever invoked from DarlingConfig.Validate() (DarlingConfig.cs:276), which is only called by DarlingWorker.ExecuteAsync (DarlingWorker.cs:566) and the CLI's validate-config command. This host loads its own copy of the config independently (config = DarlingConfig.Load() at line 146) and never calls .Validate() on it — the class doc comment even says so explicitly for the network-exposure checks: "Fail-closed, enforced HERE (the MCP host), NEVER in the all-fatal DarlingConfig.Validate (the worker's abort would not stop this host)."

So if an operator puts a connection string or password into peers.stores[].covers (or matches, or thisStoreCovers), here's what actually happens:

• DarlingWorker calls Validate(), logs the problems as critical, and returns — but that only stops the worker's own ExecuteAsync task, not the process (no Environment.Exit/StopApplication). It never reaches its Publish call, so it never sees the bad data.
• DarlingMcpHostService is a separate IHostedService in the same IHost (Program.cs:365). It keeps running, loads the same unvalidated darling.json here, and calls DarlingPeerDirectory.Publish(config.Peers) unconditionally, then renders it into ServerInstructions — which is exactly "sent verbatim to every connected MCP client" per the PR description.

The PR body states "it REFUSES to start if peer text looks like a connection string... Failing open there would broadcast the secret, which is the one place where refusing to start is proportionate" — but that guarantee doesn't hold on the actual broadcast path when mcp.enabled = true. Since the worker aborts before publishing anything, this host's unvalidated publish also becomes the only value in the shared DarlingPeerDirectory.Current, so the web dashboard's read dispatch (mentioned in the PR body as relying on the same ambient snapshot) would pick up the leaked text too.

Given this file already has a precedent for host-local fail-closed enforcement (ResolveMcpBind), this path should run PeersConfig.Validate(config.Peers) itself before Publish and fall back to DarlingPeerDirectory.Snapshot.Empty (with a LogCritical) if it finds a problem, rather than relying on DarlingConfig.Validate() having already run elsewhere.

[erikdarlingdata]

Confirmed and fixed in e35766d. I verified both halves of the premise before touching anything: there is no StopApplication or Environment.Exit anywhere in Program.cs, so the worker's return on a Validate() problem ends only its own hosted service, and this host does load and publish independently. Your point that the aborted worker never reaches its own Publish — making this host's unvalidated publish the only value in DarlingPeerDirectory.Current — is exactly right, and it's the part I'd missed.

I took it one level up from the suggestion, because this is the second instance of one category (the guard not covering a path that nobody enumerated). Rather than adding the check to this call site, DarlingPeerDirectory.Publish now validates and refuses:

internal static PublishResult Publish(PeersConfig? config)
{
    var problems = PeersConfig.Validate(config);
    if (problems.Count > 0)
    {
        s_current = Snapshot.Empty;
        return new PublishResult(Snapshot.Empty, problems);
    }
    ...
}

s_current can only be written through Publish, so the guard is now structural rather than something each publish site has to remember — a future third one cannot reintroduce the hole. PublishResult is a record struct carrying the problems so a caller cannot take the snapshot and drop the reason; both call sites log them at Critical.

A refusal publishes Snapshot.Empty — the whole block, not the valid subset. A peers block that failed validation is one the operator hasn't finished, and half a disclosure would assert coverage that may be wrong while the log says the config is broken. Losing disclosure for one restart is recoverable; leaking a credential isn't.

Harness (Windows suites can't run on macOS) reproduced the leak first, then confirmed the leaking text reaches neither the rendered instructions nor a resolution miss, and that the valid peer in a refused block isn't disclosed either. Pinned by Publish_RefusesTheWholeBlockOnAnyValidationProblem.

One thing I deliberately did not change: that the worker's Validate() abort doesn't stop the process is pre-existing for every config problem (servers must contain at least one entry behaves the same way — collection stops while MCP keeps serving a store that will never fill). That's arguably fine given each host self-gates, but it's a separate question from this PR and I didn't want to widen the diff into it.

[claude]

Reviewed. This is a clean, well-tested, purely additive C# change (no T-SQL, so the T-SQL style rules don't apply) — the "nothing declared = byte-for-byte unchanged" invariant is credibly pinned across all four surfaces, and Lite legitimately needs no twin (it has its own ServerResolver/no central-store concept, so there's no parity drift here).

One finding worth fixing before merge (posted inline on DarlingMcpHostService.cs): the "refuses to start on credential-shaped peer text" guarantee (PeersConfig.Validate) is only wired into DarlingConfig.Validate(), which only the worker's ExecuteAsync calls. DarlingMcpHostService loads darling.json independently and publishes/broadcasts config.Peers without ever validating it — so a credential accidentally pasted into peers.stores[].covers would not, in fact, be refused on the path that actually sends it "verbatim to every connected MCP client." The worker aborting doesn't stop the MCP host (separate IHostedService, same documented pattern already noted for the network-exposure checks in that file's class doc comment).

Everything else looked solid:

• The Preamble/Body split in DarlingMcpInstructions reconstructs the original Text correctly (verified the raw-string boundaries + \n\n joins reproduce the original blank-line spacing), and the cross-app census pin reads the source file directly so it's unaffected by the split.
• Null-safety around config.Peers (JSON could set it to null) is handled correctly in FromConfig/Validate/Publish.
• The ambient volatile snapshot + immutable record pattern is sound, and the test suite's try/finally reset avoids cross-test leakage.
• matches substring-matching, blank-pattern dropping, and the "no patterns declared ≠ not this peer" distinction are all correct and match their tests.

🤖 Generated with Claude Code

[claude]

Validation bypass: thisStoreCovers is never credential-checked when stores is JSON null.

var problems = new List<string>();
if (peers?.Stores is null)
{
    return problems;
}
...
/* peers.thisStoreCovers is disclosed the same way, so it gets the same guard. */
var selfText = peers.ThisStoreCovers ?? "";

The early return at line 1300-1303 fires whenever Stores is null — and the thisStoreCovers credential check lives after the loop, so it never runs in that case. Stores defaults to new() via the property initializer, but System.Text.Json will null it out on an explicit "stores": null in the JSON (as opposed to omitting the key, which leaves the default). So this config:

"peers": {
  "thisStoreCovers": "internal db, see Password=hunter2",
  "stores": null
}

passes DarlingConfig.Validate() with zero problems — the exact credential-shaped-text guard this block exists to enforce (per the PR description: "Failing open there would broadcast the secret, which is the one place where refusing to start is proportionate") is skipped, and thisStoreCovers then gets broadcast verbatim to every connected MCP client via the instructions section, list_servers' this_store_covers field, and every resolution-miss message.

None of the added tests exercise Stores == null (they all use PeersConfig's default empty-list Stores, which takes the non-null path and reaches the selfText check fine) — that's likely why this slipped through.

Suggest moving the selfText/selfOffending check above the early return (or restructuring so peers?.Stores is null doesn't skip it).

[erikdarlingdata]

Real, reproduced, and fixed in e35766d. Your exact config, run against the build before the fix:

  Stores is null after "stores": null  -> True
  Validate() problems: 0 []
  LEAK REACHES DISCLOSURE: internal db, see Password=hunter2

So System.Text.Json does assign null over the property initializer for an explicit "stores": null, the early return did skip the self-text guard, and the credential did reach FromConfig(...).ThisStoreCovers — from where all three surfaces disclose it.

You also called the cause correctly: none of my tests exercised Stores == null, because they all built PeersConfig in C# where the initializer guarantees a non-null list. The JSON path was the only way in and I never sent it a null.

Fixed by ordering, not an extra check — the thisStoreCovers guard now runs first, before anything can short-circuit, with the Stores is null return after it. An unconditional guard can't be bypassed by a shape nobody thought to enumerate, whereas a second copy of the check would just be another thing to keep in sync. The null guard also relaxed from peers?.Stores is null to peers is null so a null Stores no longer skips the whole function.

Belt and braces on top: DarlingPeerDirectory.Publish now runs PeersConfig.Validate itself and refuses (see the other thread), so even if a future guard is bypassed again, nothing invalid reaches the ambient snapshot that the disclosure surfaces read.

Pinned by Validate_ChecksThisStoreCovers_EvenWhenStoresIsExplicitJsonNull, which parses your config verbatim, asserts Stores is genuinely null (so the test still exercises the real shape rather than a fixture that can't reach it), and asserts nothing is published.

[claude]

The MCP host publishes/broadcasts config.Peers without ever calling DarlingConfig.Validate().

DarlingMcpHostService loads its own DarlingConfig independently (line 146, via DarlingConfig.Load(), which only Parse()s — it doesn't validate) and, by design, never calls config.Validate() before starting — the class doc comment even explains why: "Fail-closed, enforced HERE (the MCP host), NEVER in the all-fatal DarlingConfig.Validate (the worker's abort would not stop this host)" (lines 49-50), since network-exposure fail-closed checks are deliberately done in this host instead.

That means the peers credential guard, which only runs inside DarlingConfig.Validate() → PeersConfig.Validate(Peers), is never invoked on this path either. If darling.json's peers block contains credential-shaped text, DarlingWorker.ExecuteAsync will correctly detect it via Validate(), log it critical, and return (collection stops) — but that only stops the worker. DarlingMcpHostService is a separate BackgroundService registered in the same process (Program.cs) and keeps running regardless, reaching this line and calling DarlingPeerDirectory.Publish(config.Peers) with the same unvalidated peers block, then serving it to every connected MCP client via the instructions text, list_servers, and resolution misses.

This directly contradicts the PR description's stated guarantee: "It refuses to start on credential-shaped peer text... Failing open there would broadcast the secret, which is the one place where refusing to start is proportionate." In the actual running process, "refusing to start" only stops the collector loop — the disclosure surface (this host) starts anyway and broadcasts the secret.

Combined with the Stores == null bypass in PeersConfig.Validate (flagged separately on DarlingConfig.cs), this means the credential guard has essentially no enforcement on the code path that actually sends peer text to MCP clients. Worth having this host call config.Validate() (or at least PeersConfig.Validate(config.Peers)) and refuse to publish/expose peers on failure, mirroring the worker.

[erikdarlingdata]

Same finding as the thread above on this file — answered there in full. Short version: confirmed (no StopApplication/Environment.Exit in Program.cs, so the worker's abort ends only the worker), and fixed in e35766d by moving the guard into DarlingPeerDirectory.Publish rather than into this call site, so the ambient snapshot cannot be written without validation from any publish site, present or future. Your note that it combines with the Stores == null bypass to leave the guard with essentially no enforcement on the broadcast path was accurate — both are fixed and pinned.

[claude]

Reviewed the diff (this PR touches only C# in Darling — no T-SQL, so the T-SQL style rules don't apply here; the C# conventions in CONTRIBUTING.md are followed — nullable handling, XML docs on public/internal APIs, PascalCase/_camelCase).

Lite/Darling parity: No drift — this is a Darling-only concept (declared peer stores for a fleet split across multiple central-store boxes) that has no Lite analogue, and the PR correctly leaves Lite//Common/ untouched.

Two inline findings, both around the credential guard in PeersConfig.Validate:

1. DarlingConfig.cs — PeersConfig.Validate returns early (skipping the thisStoreCovers credential-shaped-text check) whenever peers.Stores is null rather than an empty list. An explicit "stores": null in darling.json (as opposed to omitting the key) lets a credential-shaped thisStoreCovers value sail through validation.
2. DarlingMcpHostService.cs — the MCP host loads darling.json independently and never calls DarlingConfig.Validate() (by design, per its own doc comment — network-exposure checks are deliberately done in-host instead). That means even when DarlingWorker correctly rejects a credential-shaped peers block and halts collection, the MCP host — a separate BackgroundService in the same process — still starts and broadcasts the same unvalidated peers block to every connected client.

Together, these two undercut the PR's stated design guarantee that the service "refuses to start on credential-shaped peer text" — in the actual process, that enforcement only stops the collector loop, not the surface that actually discloses the text to MCP clients. Both come with repro details in the inline comments.

Everything else checked out: the byte-for-byte-unchanged-when-empty invariant (Snapshot.Empty, Text reference equality, ResolutionMissDisclosure/EmptyRegistryDisclosure returning ""), the Preamble/Body split reproducing the original instructions text with the Fleet Coverage section correctly inserted before the tool census, the matches substring/blank-pattern normalization, and the additive (never-substituted) resolution-miss message. No missing-index DMV suggestions, no SQL injection surface (no SQL touched), no obvious performance regressions (this is a pure in-memory disclosure path, published once at startup).

[claude]

peer_note, peer_fleets, and this_store_covers are added to every list_servers response unconditionally, including the default no-peers-declared case that darling.sample.json now ships ("peers": {"thisStoreCovers": "", "stores": []}).

That contradicts the invariant this PR states in two places:

• CHANGELOG.md: "With nothing declared all four surfaces are byte-for-byte unchanged"
• DarlingPeerDisclosureTests class doc: "With nothing declared ... all four surfaces must be byte-for-byte what they were"

...and is actually pinned by ListServers_EmptyPeerFleets_SaysWhatItDoesNotProve, which asserts the new peer_note text ("No peer stores are declared...") is present even with an empty snapshot. So three of the four surfaces (instructions, resolution-miss, empty-registry prose) really are unchanged when nothing is declared, but list_servers's JSON envelope is not — every existing/default deployment now gets three new keys and a multi-sentence peer_note string on a tool response that previously had none of that.

The method doc here explains this is intentional ("peer_fleets is therefore always present and peer_note is always populated"), which is a defensible design choice on its own — but it should be called out as an exception rather than folded into a "byte-for-byte unchanged" claim, since any MCP client or script doing strict/exact JSON comparison on list_servers output will see a shape change on upgrade even if it never touches the peers config block.

[erikdarlingdata]

Fair, and fixed as a documentation correction rather than a behavior change — the claim was wrong, not the code.

You're right on all of it: three surfaces are byte-for-byte unchanged (instructions via reference equality, resolution miss via exact string equality, and list_servers' empty-registry sentence), and list_servers' JSON envelope is the one that isn't. Three new keys plus a multi-sentence peer_note land on every response, including the default the sample now ships.

The shape stays as it is, because it's the design and not an oversight: an empty peer_fleets means either "this is the only Darling store" or "the operator never declared the siblings", and this server can't tell those apart. A peer_note that only appeared once peers were declared would say nothing in precisely the case that produces the wrong conclusion — which is the conclusion #2339 was filed about. So the honest-empty note has to be unconditional to be worth having.

But an overstated claim is worse than an admitted exception, so the exception is now named in three places instead of being folded into "unchanged":

• CHANGELOG — "the PROSE surfaces are byte-for-byte unchanged ... with ONE deliberate exception: list_servers' JSON envelope carries this_store_covers / peer_fleets / peer_note on every response ... so a client comparing that tool's exact shape sees three new keys on upgrade."
• DarlingPeerDisclosureTests class doc — the negative-invariant paragraph now enumerates which three surfaces it covers, with a second paragraph for the exception and why a conditional block would be worse.
• Darling/README.md — a "Declaring nothing changes nothing, with one exception worth knowing about on upgrade" note in the new ### peers section, since that's where an operator upgrading will look.

ListServers_EmptyPeerFleets_SaysWhatItDoesNotProve is left exactly as it was. It was asserting the right thing all along; the prose around it was what disagreed with it, and a pin that contradicts the docs is the pin being right.

In c02cf1c (docs only, no behavior change).

[claude]

Reviewed the diff (T-SQL style/CONTRIBUTING.md conventions, Lite/Darling parity, correctness, security, perf). This is entirely a C# change in the Darling service — no T-SQL or Lite files are touched.

Parity: No concern. Lite has no multi-store/fleet-split deployment model to disclose peers about, and the CHANGELOG explicitly calls this out ("Lite has no peers concept and gets no twin"). CrossAppMcpToolInventoryPinTests only pins tool names, not response shapes or descriptions, so it isn't affected either.

Security: The credential-shaped-text guard is correctly structural — it lives in DarlingPeerDirectory.Publish, the one choke point both the worker and the MCP host actually go through (the MCP host deliberately never calls DarlingConfig.Validate), and a refusal drops the whole block rather than publishing a valid subset. The thisStoreCovers-ordering bug from the prior review round is fixed and has a regression test. This is local config-file input from a trusted operator, not attacker-controlled, so I don't see an injection angle.

Correctness: One finding left inline — list_servers's JSON envelope unconditionally gains this_store_covers / peer_fleets / peer_note on every response, including the default no-peers-declared deployment shipped in darling.sample.json. That's inconsistent with the PR's own stated invariant ("all four surfaces are byte-for-byte unchanged" with nothing declared) — three of the four surfaces really are unchanged, but list_servers isn't, which is worth being explicit about rather than folding into the byte-for-byte claim.

Everything else — ambient volatile Snapshot publish/read, the two-arg vs three-arg ResolveOrError overloads, null-safety around explicit-JSON-null for peers/stores/matches, and the additive (never-replacing) framing of the resolution-miss and empty-registry messages — held up under review.

[claude]

Minor security gap in this "high-signal" credential heuristic, worth a small hardening pass since a false-open here means a secret gets broadcast verbatim to every connected MCP client (the failure mode the whole Publish refusal exists to prevent):

• secretaccesskey (no separator) won't match the common env-var-style form aws_secret_access_key=... or AWS_SECRET_ACCESS_KEY, since the underscores break the substring match.
• Bearer tokens / API keys / PATs (Authorization: Bearer …, apikey=, api_key=, token=, sk-…) aren't covered at all — only DB-connection-string shapes are.

Given covers/matches/thisStoreCovers are free-text fields an operator could paste anything into, consider adding a couple more tokens (secret_access_key, apikey=, api_key=, authorization:, bearer ) to widen the net a bit. Not asking for a full secret-scanner — just noting the current list has blind spots beyond the "pasted a connection string" case it was designed for.

[claude]

Reviewed. This is a tightly scoped, thoroughly tested change (Darling-only, C#, no T-SQL touched) — 585 lines of tests pin every surface, including two review findings the PR already fixed in-flight (the thisStoreCovers validation-ordering bug that let an explicit "stores": null skip the credential guard, and moving the guard from Validate() into Publish() since the MCP host never calls Validate()).

Correctness — traced the two publish call sites (DarlingWorker.ExecuteAsync after its Validate() early-return, and DarlingMcpHostService which deliberately skips Validate()) and confirmed both funnel through DarlingPeerDirectory.Publish, so the credential guard can't be bypassed by a third call site. Verified RenderServerList's signature change lines up with DarlingDataReader.ServerListRow, the DarlingMcpInstructions.Text/Build split doesn't break CrossAppMcpToolInventoryPinTests (that test regexes the raw source file, and the census sentence still lives inside Body), and darling.sample.json stays brace-balanced after the new peers block. No bugs found.

Lite/Darling parity — the PR description states "Lite has no peers concept and gets no twin." Confirmed this is legitimate rather than drift: Lite is a single desktop app without Darling's multi-box "one store per fleet slice" deployment topology, so there's no counterpart surface for this to parity with. Not flagging it.

Security — the peers block is disclosure-only by design (no address/credential, Publish refuses the whole block rather than a valid subset on any credential-shaped text), which is the right call given it's broadcast verbatim to every connected MCP client. Left one inline comment: the CredentialShapedTokens heuristic has a couple of blind spots (underscore-separated aws_secret_access_key style names, bearer tokens/API keys generally) — minor, since the design explicitly disclaims being a full secret scanner, but worth a small hardening pass given the blast radius of a false negative.

No performance concerns (this is all in-memory config/string work, not a hot path) and no other findings.

[erikdarlingdata]

Reviewed the diff, not the report — merging. I checked the three things that could have made this dangerous rather than useful:

1. The census pin survived. The instructions still read This server exposes 101 tools. 76 are the same names, so CrossAppMcpToolInventoryPinTests still parses it. Inserting a Fleet Coverage section above the census was the risky placement and it held.
2. It really is disclosure-only. Grepping DarlingPeerDirectory for address/credential/endpoint/connection turns up nothing but doc comments explaining their absence. Tier 1 stayed tier 1.
3. The default install is untouched. disclosure.Length == 0 ? message : ... means a store declaring no peers produces a byte-identical miss message. That was my main regression worry — a fleet-split feature that changes what every single-store operator sees — and it is explicitly guarded and pinned.

The credential-guard bug you found and fixed mid-review is the good catch here: DarlingConfig.Validate is run by the worker, whose failure path is a return rather than a process exit, and the MCP host loads its own config and never validates — so the one path that actually broadcasts peer text to clients was the one path the guard never covered. Moving it inside Publish is the right shape, since that is where the broadcast is.

Also noted: you corrected your own "all four surfaces byte-for-byte unchanged" claim rather than leaving it standing when list_servers always carries the three new keys. That distinction matters to anyone parsing the envelope.