Fix --configure-network comma splice after string-valued last members (#2073)

[erikdarlingdata]

Closes #2073.

What

--configure-network produced invalid JSON ("dataDirectory":, "…") when the target section's last member is a string with no trailing comma — the write-time parse gate then refused to write, so the wizard simply could not configure LAN exposure for files of that shape (no on-disk corruption ever occurred).

Root cause (as diagnosed in the report — verified against the classifier before fixing)

InsertAsLastMember located the previous member's end via PreviousCodeIndex, a code-only backward scan. A string value is entirely StringLiteral region — quotes included — so the scan skipped the whole value, landed on the member's colon, and spliced the synthesized comma there. Only a string-valued, comma-less last member trips it: objects/arrays/primitives end in Code characters, and a trailing comma is Code and found first — which is why the shipped sample's fixtures never exercised the shape.

Fix

New PreviousMemberEndIndex: same walk, but a StringLiteral hit terminates it too — that hit is always a real value's closing quote (quoted runs inside comments classify as Comment). The comma now lands after the value. The other PreviousCodeIndex call site (member removal) only ever looks for ,/{, both always Code — checked, not affected.

Tests

Regression fixture is the reporter's exact minimal-config repro: parses through DarlingConfig.Parse (the wizard's write gate), asserts the comma rides after the value and the :, shape never appears, and confirms the block is live through the real ResolveNetworkExposure.

🤖 Generated with Claude Code

[claude]

Nit: downstream of this call, lastCodeIdx/lastCode (a few lines below) are now stale names — the whole point of this fix is that the index can land on a StringLiteral closing quote, not just Code region. Worth renaming to something like lastMemberEndIdx/lastMemberEndChar so a future reader doesn't assume "Code" and reintroduce the #2073 assumption elsewhere. Not a functional issue — the != ',' check is correct regardless of region.

[claude]

Review

What it does: Fixes a real bug in the --configure-network JSON editor. InsertAsLastMember located the previous member's end via a code-only backward scan (PreviousCodeIndex), which treats an entire string value — quotes included — as non-code and skips over it. For a comma-less, string-valued last member, the scan landed on the member's colon instead of after the value, producing invalid JSON ("dataDirectory":, "..."). Since the wizard parse-validates before writing, the practical symptom was a refusal to write rather than on-disk corruption, but LAN exposure could never be configured through the wizard for a config of that exact shape.

Fix: New PreviousMemberEndIndex terminates the backward walk on either a Code-non-whitespace hit (existing behavior) or a StringLiteral hit, since the first StringLiteral character found scanning backward from the object's close is always the value's closing quote (verified against Classify — the closing quote itself is labeled StringLiteral, and quoted runs inside comments classify as Comment, not StringLiteral). InsertAsLastMember now calls this instead of the old code-only scan.

Correctness

• Traced the fix against Classify's state machine — the closing-quote labeling and escape handling (\", \\) confirm a string's last StringLiteral char, scanning backward, is always its true closing quote, never a false match on an escaped inner quote or a comment containing quotes.
• Checked the other call site of the old PreviousCodeIndex (member removal, line ~393) — it only tests json[beforeIdx] == ',' for an already-adjacent separator and never needs to skip over a value to find one, so it's correctly unaffected, matching the PR description's claim.
• Verified the fix doesn't regress non-string last-member cases (objects/arrays close on }/], primitives end in Code) — the new function preserves the old Code-based branch verbatim for those.
• Verified empty-object handling (lastCodeIdx == objectOpen) and existing-trailing-comma detection are unaffected by the added StringLiteral branch.

Test coverage

Good regression test: parses through the actual write gate (DarlingConfig.Parse), checks both the presence of the correctly-placed comma and the absence of the :, splice shape, and confirms the resulting network block is live via ResolveNetworkExposure — not just syntactically present. Matches the reporter's exact minimal repro.

Minor/optional: the fix lives in shared logic used by all three sections (postgres/mcp/web) and by the top-level section-insert call site, so one test is enough to cover the code path — but a second case (e.g. mcp/web, or a value with a trailing block comment before the brace) would tighten confidence further. Not blocking.

Lite/Darling parity

No parity concern — --configure-network and DarlingNetworkConfigEditor are Darling-only; grepped for any equivalent JSON-editing logic under Lite//Lite.Tests/ and found none, so there's no counterpart that needs the same fix.

Style / docs / changelog

XML doc comments on the new private helper are consistent with the rest of the file (which documents private helpers too, beyond the "public APIs only" guidance in CONTRIBUTING.md). CHANGELOG entry is well-written and consistent with existing entries' format, and the [#2073] reference link was added correctly.

Left one inline nit (non-blocking): lastCodeIdx/lastCode in InsertAsLastMember are stale names now that the index can land on a StringLiteral closing quote rather than only Code region.

Overall: solid, minimal, well-reasoned fix with a real regression test tied to the actual bug mechanism. No security, correctness, or performance concerns.