Parse the crude second opinion's XML instead of guessing its quote delimiters

[erikdarlingdata]

Closes #3140.

ParsedProjectXmlPaths is the deliberately-crude second opinion that TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds floors MSBuild's evaluated item set against. It read the project XML with a regex alternation whose single-quote arm treated an apostrophe anywhere — in a comment, in prose — as an attribute delimiter: it paired with the next apostrophe and every literal between them left the answer. That is an under-read of arbitrary literals, which is precisely what the doc claimed twice it could not do.

What was swallowed on dev

Measured with the shipped read, not reasoned about:

project	apostrophes	swallowed span	attribute values invisible
Lite.Tests/Lite.Tests.csproj	3 (odd)	139 chars, offsets 3714–3853	the whole <None Include="Fixtures\SystemHealth\*.xml" CopyToOutputDirectory="PreserveNewest" /> item
Darling/Darling.Tests/Darling.Tests.csproj	6 (even)	2,046 chars, offsets 3311–5357	four <None> items — DarlingStoreUpgrade.cs, Mcp\DarlingMcpHostService.cs, Mcp\DarlingWebHostService.cs, ..\tools\fetch-pg-runtime.ps1 and each one's Link

Neither file's swallowed region held a cross-app path, which is the only reason the guard was green — green by position, not by correctness. And the file that lost the most has an EVEN apostrophe count, so a check on parity would have called it clean. Position relative to the item is the invariant; parity never was.

The fix, and what was rejected

LiteralXmlValues reads the document with XDocument and yields each attribute value and each element's text. Crude now means UNEVALUATED — no property expansion, no globbing, no Conditions — rather than mis-parsed, and the quoting grammar is the XML parser's problem, because a regex that gets XML quoting right is most of an XML parser. XDocument.Parse on a .csproj is already this repo's idiom (WriteLockBudgetTests).

A comment's body is still text-scanned by CommentedOutValues, for double-quoted values and >-to-< spans, so a path inside a comment still counts. That over-read is the position ImportElements takes for the same stated reason — a commented-out construct is a construct arriving unnoticed — so a reader that skipped comments would have narrowed a documented guarantee while claiming to fix a bug. Dropping only the apostrophe arm there is what makes prose harmless; the price is a single-quoted attribute inside a comment, which is now stated in the doc and asserted.

Rejected:

• Strip XML comments before scanning. Does not fix it — an apostrophe outside a comment is the same defect — and it narrows the one over-read the doc and ImportElements both rest on.
• Delete the single-quote arm. MSBuild accepts Include='…', so this trades a silent under-read of arbitrary literals for a silent under-read of single-quoted ones. XDocument reads that spelling correctly, and the pin asserts it.
• Fail when quote pairing does not close. An odd count is not the failure condition: Darling.Tests.csproj was even and lost four items, and Condition="'$(X)' != ''" carries four apostrophes inside one correct value. A parity check fails toward "clean" in the case that matters.

The floor assertions now name which side lost the path

Assert.Contains(floor, parsedCross) and Assert.Contains(floor, evaluated) sat adjacent and message-less, so a crude-parse failure presented as an evaluated-side regression — which is how #3138's red was read. Each is now an Assert.True naming its own side. The crude one stays first because it is the precondition, and it says so.

Reproduced both directions: with dev's file plus one apostrophe each side of the floor item, the failure was Assert.Contains() Failure … Set: [] … Not found: "Darling/Darling.Tests/CSharpSourceWalker.cs". The same scenario now says the CRUDE parse is broken and that Evaluate is not implicated; dropping Compile from EvaluatedItemTypes instead says the EVALUATED read has lost the item.

The pin

TheCrudeParse_SeesEveryLiteralValueHoweverItIsQuoted states the property — every path the XML names literally is in the answer, and the only misses are values MSBuild has to evaluate — then enumerates the one way it can become false: something in the text is taken for a delimiter. Each route gets its own document and its own file name, so a broken route reds by name rather than hiding behind a sibling. Both negative arms carry a literal in the same document asserted PRESENT, so an absence cannot be the absence of a read. Nothing touches the disk; the read resolves and anchors by path arithmetic.

Lite.Tests gained no build.yml filter entry and needs none.

Changelog entry for the coordinator

- **The crude second-opinion parse in `CrossAppGuardCiGateTests` no longer loses project items to an apostrophe** ([#3140]) - `ParsedProjectXmlPaths` is the deliberately-crude text scan that MSBuild's evaluated item set has to be a superset of, and its single-quote alternation treated an apostrophe anywhere in the file - including inside an XML comment - as an attribute delimiter. It paired with the next apostrophe and every literal between them vanished from the floor, so the comparison quietly measured less than it claimed. Live on `dev`: one `<None>` item hidden in `Lite.Tests`' own project and four (about two kilobytes of markup) in `Darling.Tests`', which carries an EVEN apostrophe count - parity was never the invariant, position relative to the item was. The read now parses the document with `XDocument`, so crude means unevaluated rather than mis-parsed, while a comment's body is still text-scanned for the documented over-read that makes a commented-out path count. The two doc paragraphs claiming this could only under-read what needs evaluation are corrected and are now held by a pin. The floor assertions also name which side lost the path, so a crude-parse failure stops presenting as an evaluated-read regression the way it did in [#3138].

[claude]

Reviewed. This is entirely test-infrastructure (Lite.Tests/CrossAppGuardCiGateTests.cs) — no T-SQL, no production Lite/Darling code, so the SQL style and Lite/Darling parity conventions don't apply here beyond the file itself, which already treats both SKUs symmetrically via the own/other tree parameterization.

Verified the core claim against the actual repo files rather than taking the PR body's numbers on faith:

• Darling/Darling.Tests/Darling.Tests.csproj has 6 apostrophes (even), and the pairing that matters is the one spanning the two comments around lines 47–68 — it swallows the real (uncommented) <None> items for DarlingStoreUpgrade.cs, DarlingMcpHostService.cs, DarlingWebHostService.cs, and fetch-pg-runtime.ps1 in between. Confirms the PR's point that apostrophe parity was never the invariant — position relative to the item was.
• Lite.Tests/Lite.Tests.csproj has 3 (odd), pairing across lines 57/59 and swallowing the Fixtures\SystemHealth\*.xml item, also as described.

Traced LiteralXmlValues/CommentedOutValues against every arm of the new TheCrudeParse_SeesEveryLiteralValueHoweverItIsQuoted test by hand (single/double-quoted attrs, element text, CDATA, entity-encoded separators, apostrophes inside a real Condition, commented-out quoted/text spellings, the single-quoted-inside-a-comment residual miss, and the malformed-XML throw path) — the implementation matches the documented behavior in every case I checked, including the subtlety that each XComment is parsed as an isolated node so the old cross-comment regex leak structurally can't recur.

No correctness, parity, security, or performance issues found. Left one inline nit — a doc-wording imprecision in a summary line (LiteralXmlValues), not a functional bug.

[claude]

Review

Scope: single file, Lite.Tests/CrossAppGuardCiGateTests.cs — replaces the regex-based "crude second opinion" XML scan (ParsedProjectXmlPaths) with an XDocument-based one (LiteralXmlValues/CommentedOutValues), tightens the floor assertions in TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds to name which side lost the path, and adds a new pinning test TheCrudeParse_SeesEveryLiteralValueHoweverItIsQuoted.

Not applicable here, checked and ruled out:

• T-SQL style (OPTION(RECOMPILE), AS aliasing, etc.) — no T-SQL in this diff.
• Lite/Darling parity — CrossAppGuardCiGateTests is intentionally Lite.Tests-only (it's the guard that checks both apps' test projects from one place); there's no Darling-side counterpart file that needs a matching change.
• Security — no external input, network, file, or process use beyond an unwritten temp path used only for lexical Path.GetFullPath resolution (per the comment at the new test, "Nothing here touches the disk," which I verified: Absolute/CrossAppPath operate purely lexically, no I/O against repo).

Correctness walkthrough:

• LiteralXmlValues visits document.DescendantNodes() and switches on XElement (attributes), XText (covers XCData too, since XCData : XText), and XComment (delegates to the reduced CommentedOutValue regex, apostrophe arm removed). This structurally fixes the reported bug: apostrophes in prose/comments can no longer pair across sibling comments and swallow real <ItemGroup> content between them, because comments are now parsed per-node instead of via a single regex pass over the whole raw text.
• The new pinning test's arms check out against the implementation: single-quoted attributes (Include='...'), entity-encoded separators, CDATA bodies, and apostrophes inside Condition="'$(X)' != ''" are all correctly resolved through XDocument, and the one documented residual gap (a single-quoted attribute inside a comment) is real and correctly asserted as a miss — CommentedOutValue only has a double-quoted arm and a >...< arm, so it can't recover that spelling.
• Malformed XML now throws XmlException uncaught rather than silently answering short, which is the correct call for a floor/guard — asserted via Assert.Throws<XmlException> on an intentionally unclosed element.
• The Assert.Contains/Assert.DoesNotContain → Assert.True/Assert.False swap is a pure diagnostics improvement (adds which-side messages); no behavior change.

Caveat: I could not execute dotnet build/dotnet test in this review environment (sandboxed, no permission to run dotnet), so this is a static read rather than a verified green run. Worth confirming CI is green on this PR, particularly the new TheCrudeParse_SeesEveryLiteralValueHoweverItIsQuoted fact and the existing TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds/TheCSharpMatchers_StillCannotSeeTheseSpellings tests it neighbors.

No correctness, parity, or security issues found.

[claude]

Reviewed the diff (Lite.Tests/CrossAppGuardCiGateTests.cs only).

Scope check: no T-SQL touched, so the T-SQL style rules don't apply here. No Darling.Tests counterpart exists for this file, and none is needed — CrossAppGuardCiGateTests is explicitly Lite-only CI-gate infrastructure per CONTRIBUTING.md's "Writing a Test That Reads the Other SKU's Source" section, so this isn't a Lite/Darling parity gap.

Correctness: traced the new LiteralXmlValues/CommentedOutValues helpers against each Sees/DoesNotSee arm in TheCrudeParse_SeesEveryLiteralValueHoweverItIsQuoted:

• Attribute values now come from XElement.Attributes() per-element rather than a document-wide regex, so an apostrophe in a sibling Condition attribute (or in prose in element text) can no longer bleed across element boundaries and swallow an unrelated Include — that was the root cause of ParsedProjectXmlPaths treats an apostrophe in an XML comment as an attribute delimiter, so the crude second-opinion scan silently under-reads — and its doc claims twice that it cannot #3140, and it's structurally impossible now.
• XCData : XText is handled by the shared case XText arm, correctly picking up CDATA-delivered paths.
• The comment-body regex (CommentedOutValue) drops the single-quote arm entirely, so pairing across two separate <!--...--> comments (the exact ParsedProjectXmlPaths treats an apostrophe in an XML comment as an attribute delimiter, so the crude second-opinion scan silently under-reads — and its doc claims twice that it cannot #3140 failure mode) can't happen — each XComment is scanned independently now.
• Manually walked the regex against the CommentedDouble/CommentedSingle fixture: the double-quoted value matches via the quoted arm, the single-quoted one falls through unmatched (no apostrophe arm), matching the asserted DoesNotSee. Same walk confirms CommentedText.cs's element-text-in-comment case via the >text< arm.
• The deliberately-uncaught XmlException on a malformed project (no fallback swallowing a bad parse into a silent empty answer) matches the stated "fail loudly" design goal and is exercised by the new Assert.Throws case.
• XDocument.Parse with no custom XmlReaderSettings defaults to DtdProcessing.Prohibit, so this doesn't introduce an XXE-style risk when parsing PR-supplied .csproj content in CI.

No security, performance, or parity issues found. The doc-comment rewrites on ParsedProjectXmlPaths/TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds accurately describe the new behavior and match what the code does. This looks correct and well-covered — nothing blocking.