Fixes #3074

[erikdarlingdata]

Closes #3074.

The project-file stage of CrossAppGuardCiGateTests evaluated MSBuild by hand. #3065 fixed seven correctness findings in it during review, all of them real, and the next constructs were already visible — Condition= on an item group, Choose/When, imported props that define the item being read. This replaces the stage with dotnet msbuild -getItem -getProperty, which answers the same question fully evaluated.

The .cs string-literal half is unchanged. This replaces the project-file half only.

The invocation, re-measured on this branch

dotnet msbuild <project> -getItem:Compile;None;Content;EmbeddedResource;Reference;ProjectReference -getProperty:MSBuildProjectFullPath;DirectoryBuildPropsPath;DirectoryBuildTargetsPath;DirectoryPackagesPropsPath -p:EnableWindowsTargeting=true

Measured on a fresh worktree of origin/dev that had never been built, sandboxed, no network:

question	result
Exit code	0
Elapsed	0.30 s (Lite.Tests), 0.34 s (Darling.Tests)
obj/ created	none, before or after
Items and Properties in one invocation	yes — they combine, and -getItem returns {"Properties":{…},"Items":{…}}

Two corrections to the issue's own measurements, both verified here:

• -p:EnableWindowsTargeting=true is not required for evaluation. dotnet msbuild -getItem on Lite.Tests (net10.0-windows, UseWPF) exits 0 on macOS without it; only a build needs it. It is passed anyway — it is a no-op on the Windows runner and removes a host-dependent difference from a guard that has to answer identically in CI and on a macOS verification run.
• -preprocess and -getItem do not combine (-getItem wins silently), so the import list is taken from -getProperty instead. That turned out better: see the Directory.Packages.props finding below.

The superset proof

Run both routes over the real tree. The project-item stage's contribution to the found set, before and after:

arm	old route (regex)	new route (-getItem)
Lite.Tests → Darling	Darling/Darling.Tests/CSharpSourceWalker.cs	Darling/Darling.Tests/CSharpSourceWalker.cs
Darling.Tests → Lite	none	none

Darling.Tests.csproj names no file under Lite or Lite.Tests at all, so its cross-app project answer is legitimately empty on both sides — which is why the superset pin can only floor that arm on its own-tree answer.

And the whole found set, both stages together, is identical. Measured by compiling origin/dev's version of the file and this branch's version into two harnesses side by side, driving the shipped Scan through reflection in each, and diffing the results — not by eyeballing a tally:

• pre-change: 41 references (22 on the Lite arm, 19 on the Darling arm)
• post-change: 41
• comm -23 (in before, not after — a regression if non-empty): empty
• comm -13 (in after, not before): empty

The only difference anywhere is the attribution string: the linked compile now reads Lite.Tests/Lite.Tests.csproj (Compile FullPath) instead of a bare file path, because origin comes from MSBuild's DefiningProjectFullPath.

Nothing is missed, and the comparison is a live assertion rather than a one-time measurement — TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds keeps a deliberately crude XML parse as a second opinion and requires the evaluated set to contain it.

The one thing the evaluated read does not report, and what happens instead

An <Import>'s own path. Items an imported file defines arrive with everything else — proven in the fixture — but the imported file's path is not an item and no -getProperty answer carries it. -getProperty covers the build files MSBuild locates itself; an arbitrary hand-written <Import Project="..\Darling\Shared.targets" /> is not among them.

There is no <Import> element in any MSBuild file in this repository, and the old route's handling of it was pinned entirely against synthetic XML for that reason. Enumerated rather than tallied: every one of the 24 tracked files matching *.csproj, *.props, *.targets, *.proj, *.sln, *.projitems or *.shproj was grepped individually and printed — zero occurrences of <Import, and zero for a case- and whitespace-insensitive <\s*[Ii]mport as well. The 22 hits for Sdk= are the SDK-resolution construct, which resolves through a resolver rather than to a path and is deliberately not what the gate matches — asserted as a negative in the gate's known-answers pin. So this is a gate rather than an evaluator: Check fails loudly if a scanned project — or a build file MSBuild says it imported — grows one. Three lines, versus a rule per construct.

A gap the old route had that this closes

Directory.Packages.props is imported into every project here, and it is a plain MSBuild file: an ItemGroup in it becomes an item group of every project. The old route probed upward for exactly two names, Directory.Build.props and Directory.Build.targets, so it would never have opened it — and its ImportedBuildProbes floor asserted it had looked "in the places MSBuild would probe" while omitting one MSBuild does probe. -getProperty:DirectoryPackagesPropsPath returns it, and any item it defines is in the -getItem answer regardless.

How the shell-out fails loudly

A guard that reports "no cross-app references" because a process never started is worse than the regex it replaced. Four ways of producing an empty answer without looking, each floored:

1. The process never started. RunDotnet catches Win32Exception and returns exit -1 with the OS message. (DOTNET_HOST_PATH is not set for a process launched by dotnet run, which is how this suite runs, so dotnet is launched by name from PATH — measured, not assumed.)
2. It exited non-zero. Evaluation.ExitCode, asserted on its own.
3. Its output did not parse. ParseEvaluation is strict — a banner, an error summary, an empty stream or a JSON object with no Items is a failure, not zero items — and returns the reason as Evaluation.Failure, asserted separately from the exit code.
4. MSBuild answered about an item type it does not recognise. -getItem:Compile;Bogus exits 0 with "Bogus": []. Measured. Key presence proves nothing, so the item-type list is floored per type — and it cannot be floored against this repository, where Content, EmbeddedResource and Reference are all legitimately empty.

The per-type count comes from MSBuild's own array lengths, not from the paths the extraction pulled out of them, because those answer different questions: the count is evidence the type name is real, and what the extraction found is asserted separately by the found set. They also genuinely diverge — one item can yield two paths, a Reference carrying a HintPath being exactly that, so a derived count read as two Reference items where the fixture declares one. Mutations 1 and 2 now fail on the two claims independently: a typo in the list shows References=0 in the counts, while a broken HintPath read leaves Reference=1 and the path missing from the found set.

AssertInvocationSucceeded carries (2), (3) and a distinct Items.Count > 0 floor, and is read from Check as well as from the pins so the two cannot drift. A hang is bounded by a two-minute deadline with Kill(entireProcessTree: true); both streams are drained concurrently, because reading one to completion first deadlocks on a half-megabyte answer.

The synthetic fixture, which is where HintPath and the item-type list are proven

There is no Reference item anywhere in this repository, so the HintPath route to a cross-app assembly has no live example — nor is there a Condition, a Choose, an imported props file, a transform, or a cross-app wildcard. WriteCrossAppFixture writes a miniature two-SKU tree to a temp directory (consuming project under one SKU's test tree, targets under the other SKU's app tree and its sibling test tree, so #3067's anchor is exercised on this population too) carrying one item of every requested type and one instance of every construct:

construct	holding arm	non-holding arm
$(Property) in an Include=	FromProperty.cs found	—
HintPath, written relatively, on a Reference	Hinted.dll found	—
Condition= on an ItemGroup	ConditionTrue.cs found	ConditionFalse.cs absent
Choose/When/Otherwise	ChosenWhen.cs found	ChosenOtherwise.cs absent
item defined in an imported .props	FromImport.cs found, attributed to Imported.props	—
@(Item->'…') transform	Transformed.cs found	—
wildcard	Globbed.xml found	NotGlobbed.txt absent
root-prefix anchor	—	DarlingExtra/Nope.cs absent

Each construct is asserted in both directions where it has two, so a fixture that failed to write its tree cannot pass by finding nothing.

Imported.props sits above the consuming project rather than beside it, and carries both the Reference and one of the Compile items. That is load-bearing, and it is a defect this PR found in itself:

A relative HintPath needs resolving, because MSBuild returns that metadata as written (unlike FullPath). I first resolved it against the directory of the file that declared the item. That is wrong, and measuring it settled the question: a Reference in a Directory.Build.props one directory up, whose relative HintPath names a file beside that props file, does not resolve — -t:Restore,ResolveAssemblyReferences -getItem:ReferencePath reports nothing; the same HintPath naming a file beside the consuming project resolves to it. So the base is the project directory, from MSBuildProjectFullPath.

The first version of the fixture could not have caught this, because it declared the Reference in the project file itself, where the two candidate bases are the same directory. Moving the imported file up one level makes both bases divergent: written from there, the relative Include and the relative HintPath resolve out of the fixture tree under the wrong rule. Mutation 13 below is the proof — it reds now and would have passed before.

Getting that backwards is a silent miss in exactly this guard's direction: the path resolves to something real-looking, fails the cross-app anchor, and vanishes.

One correction for anyone reading the automated review summary alongside this. It states that HintPath "now resolves against DefiningProjectFullPath's directory rather than the consuming project's directory — correct per MSBuild's own resolution rules for metadata declared in an imported file." That is backwards on both counts: it is not what the code does, and it is not MSBuild's rule. ParseEvaluation resolves against Path.GetDirectoryName(Properties["MSBuildProjectFullPath"]); DefiningProjectFullPath is used only for attribution and never for resolution. The measurement in the table above is what settled it, and mutation 13 is the standing guard — swapping the base back reds the fixture pin. Flagging it here rather than in a comment because the summary is otherwise accurate and this is the one line in it that, if believed, would send someone to reintroduce the defect.

Anti-vacuity floors

• Check: CSharpFiles > 0, ProjectFiles.Count > 0, Evaluated non-empty, AssertInvocationSucceeded per evaluation, UnreportedImports empty, and CrossAppGuardCiGateTests anchors on Lite/, so the two Darling.Tests reads of Lite.Tests/ are invisible to it #3067's existing floor on the other SKU's test tree.
• Population pin: the manifest's own evaluation located by name (not by position); Compile > 0 and ProjectReference > 0 — the two types a test project cannot honestly be empty of; MSBuildProjectFullPath equal to the project that was asked, which is how "MSBuild found no Directory.Build.props" is distinguished from "no properties came back"; and the linked compile required by name with its attribution.
• Superset pin: the crude parse must find something under the arm's own trees, because superset of nothing is free; plus the Lite arm's cross-app answer floored by name on both sides.
• Import gate: known answers in both directions, because Check's emptiness assertion holds on this tree whether or not the detector works.

Every project file under a test tree is evaluated, not projectFiles[0] — there is one today, and taking the first would have gone on answering about whichever sorted first the moment a second appeared.

Red-proof evidence

Every mutation was applied to a committed tree, rebuilt with the build result checked separately from the run, and restored by git checkout -- with the restore verified by grepping for a MUTANT_MARKER token rather than by trusting a diffstat. Every variant compiled (0 Error(s) + Build succeeded) and every one changed the reported outcome, so none is a variant that never ran.

And the baseline was re-run LAST, after all seventeen: empty git status, empty git diff HEAD, no MUTANT_MARKER anywhere in the worktree, and both harnesses green. A clean baseline beforehand only proves the tree was clean then; this is the one that proves nothing leaked between cases.

#	mutation	what went red	how the failure named its own claim
1	EvaluatedItemTypes: Reference → References	fixture pin only	the fixture declares one References item and MSBuild returned none of that type … Counts: [Compile=5, Content=1, EmbeddedResource=1, None=2, ProjectReference=1, References=0]
2	drop the HintPath metadata read	fixture pin only	Assert.Contains failed on Darling/PerformanceMonitor.Darling.Service/Hinted.dll, with Reference=1 still in the counts — the extraction claim failing while the type-name claim holds
3	fixture FixtureGate flipped to no	fixture pin	Assert.DoesNotContain found ConditionFalse.cs, and the reported set showed ChosenOtherwise.cs had also appeared — both condition arms are live
4	Evaluate swallows a non-zero exit and reports success	loud-failure pin only	Assert.NotEqual on the exit code — nothing on the real tree catches this, which is why the pin exists
5	ParseEvaluation returns zero items with no failure	4 tests, incl. EveryCrossAppSourceRead_…	the evaluated project read returned no items at all, which no project does
6	CrossAppItems silently skips Compile items	3 tests	superset pin: Set: [], Not found: Darling/Darling.Tests/CSharpSourceWalker.cs; population pin named the missing attribution
6b	add <Compile Include="..\Darling\…\CSharpSourceWalkerTests.cs" Condition="false" /> to Lite.Tests.csproj	superset pin only	does not contain what a parse of the same XML sees: Darling/Darling.Tests/CSharpSourceWalkerTests.cs … Evaluated: [Darling/Darling.Tests/CSharpSourceWalker.cs] — proves the comparison itself, not just its floor
7	ParsedProjectXmlPaths yields nothing	superset pin	the XML parse found no path under Lite.Tests's OWN trees … every comparison below is superset-of-nothing
8	ImportElements never matches	import gate only	Assert.Equal collections differ — Check's emptiness assertion stayed green, which is exactly why the gate needs known answers
9	properties returned as empty strings	population pin	Assert.Equal on MSBuildProjectFullPath
12	ProcessStartInfo("dotnet-not-a-real-executable")	5 tests	exited non-zero … stderr=dotnet could not be started: … No such file or directory — including EveryCrossAppSourceRead_…, the pin whose success condition is emptiness
13	resolve HintPath against the declaring file's directory instead of the project's	fixture pin only	Assert.Contains failed on Darling/PerformanceMonitor.Darling.Service/Hinted.dll. This is the mutation the first fixture could not catch, and the reason the imported props file was moved above the project
14	fixture Imported.props moved back beside the project	fixture pin	error MSB4019: The imported project … "../Imported.props" … was not found — the fixture fails loudly rather than silently losing an item, so its own layout is not a silent assumption
15	the timeout branch returns (0, "", "") instead of failing	loud-failure pin only	Assert.NotEqual on the exit code — the deadline's own claim, which had no pin at all until the second review thread
16	WaitForExit ignores the injected deadline and uses the shipped constant	loud-failure pin only	same assertion — so the pin measures the deadline that was passed, not one it cannot influence
17	drop the ValueKind guard on the property read	loud-failure pin only	InvalidOperationException : The requested operation requires an element of type 'String', but the target element has type 'Number' raised out of ParseEvaluation — the exact bypass of the Failure floor that the third review thread identified

1, 2, 4, 6b, 8, 13, 15, 16 and 17 each red exactly one test, and that is the point: each names a defect nothing else in the suite can see. 12 is the failure mode the issue called strictly worse than the regex, and it now reds five tests with the command line in the message.

Note on 6b: a Condition="false" cross-app include makes the crude parse over-read and the pin red. That is deliberate — the failure message says so and asks a human whether that path is a read worth covering, rather than silently taking either route's word for it.

Verification

• Lite.Tests is net10.0-windows with UseWPF and cannot run on macOS, so the actual CrossAppGuardCiGateTests.cs was compiled into a throwaway net10.0 xunit v3 console harness staged inside the gitignored Lite.Tests/bin/, and run there. Green: Total: 10, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0. Microsoft.Testing.Platform prints no per-test Passed lines, so that summary and the 12 mutations above are the evidence, not a grep for failures.

• The harness reads the edited file rather than a stale binary: the runtime moved from 0.30 s to 3.2 s on the switch (the invocations), the test names changed, and every mutation above changed the outcome.

• dotnet build Lite.Tests/Lite.Tests.csproj -p:EnableWindowsTargeting=true succeeds on macOS: 0 Error(s), 9 warnings, none in either changed file. That compiles the whole project including all 300 test files, not just the one under change.

• Re-run with build output present, so the bin/-and-obj/-exclusion assertion is exercised against real artifacts: three throwaway .csproj files were sitting in Lite.Tests/bin/ and the population pin's Assert.DoesNotContain(… "/bin/" …) held. Still green.

• No obj/ created under either scanned project by the guard, and no fixture temp directory left behind.

• The Darling.Tests arm was verified too, and it is the arm I nearly left unchecked. Two guards there read the file this PR changes and cannot run on macOS either: DocCommentHygieneTests scans every .cs file under the repo root (stacked <summary> blocks, unbalanced doc runs), and CommentFilterAdoptionTests scans both test projects for an unregistered line-prefix comment filter and compares the result for equality against its own map. Both were compiled into a second net10.0 harness with CSharpSourceWalker.cs and run: Total: 42, Errors: 0, Failed: 0, Skipped: 0, Not Run: 0.

  That harness was red-proofed against this PR's own file, twice, because a whole-repo scanner reporting clean is exactly the shape that reports clean on the wrong tree: injecting a stacked <summary> into CrossAppGuardCiGateTests.cs reds NoMemberCarriesTwoStackedSummaryBlocks, and injecting a line.TrimStart().StartsWith("//", …) into it reds EveryLinePrefixCommentFilter_StatesTheBoundItRestsOn naming Lite.Tests/CrossAppGuardCiGateTests.cs as the unregistered site. So the 42/0 is evidence about this diff rather than about a scan that never reached it.

  For the record, the shipped code's several .StartsWith calls take path prefixes rather than comment prefixes, which is why the adoption scan is silent on them — the detector requires a comment-prefix literal at the call site.

Also in this PR

CrossAppMcpToolInventoryPinTests' summary cited a "55-vs-73 drift". The numerals narrate why the pin exists rather than asserting anything the code reads, so per #3072's three-class policy they are deleted rather than corrected, and the sentence now says the same thing without a number.

Deleting was the right class on the merits and it is also the one immune to merge order: #3078 adds an MCP tool, which moves the count again. For the record the figures were re-derived by running the shipped ExtractToolNames through reflection at 0f888e90c: 87 Lite, 138 Darling — not the 87/137 quoted to me. Nothing in this PR depends on either figure.

Review findings, and one more the review found

Two findings, both correct, both fixed:

1. using System.Security; was unused. It was for a SecurityElement.Escape call in an earlier fixture that interpolated the repository's absolute path into the generated XML; the fixture writes relative MSBuild properties instead, so there is nothing to escape. Dropped in 8b5122b85, replied in the thread, thread resolved.

2. CSharpPaths' summary still pointed at MsBuildPaths, a method this change deletes. Real rot, and it is worth saying why nothing caught it: GenerateDocumentationFile is off for this project, so the compiler never binds a cref and a deleted member leaves its inbound doc references dangling in silence. Fixed in 330f6958f by stating the reason directly instead of cross-referencing.

   Rather than fix only the one that was reported, I built the project once with -p:GenerateDocumentationFile=true and let the compiler enumerate them all. Result: 22 distinct CS1574 across the whole Lite.Tests compilation, none in either file this PR touches — so mine is the only one this change introduced, and it is gone. (Roughly half of the 22 look like genuinely stale member references — AppliesTo five times, PayloadColumns, IsVictim, WaitTime — and the rest name types in assemblies Lite does not reference, which a cref cannot resolve by design. The split needs triage rather than a number, and it is pre-existing either way.)

Rebased? No — merged twice, and every figure re-derived after each

dev moved five times during review. Two of those touch this PR's files, and origin/dev was merged in both times rather than rebased, since a rebase means a force-push:

• Fixes #2880 #3078 (Out-of-band watchdog: sample server-wide waits during a collector stall (the per-session capture cannot be built — target_session_id is unpopulated) #2880) touches Lite.Tests/CrossAppMcpToolInventoryPinTests.cs. Merged at 663de9b36. No conflict, and none possible — the numerals were deleted from that summary rather than corrected, so there was no figure left for a +1 to the Darling tool count to make wrong.
• Fixes #3076 #3079 (CrossAppGuardCiGateTests cannot see a cross-app read spelled as a bare directory name, and there is a live one #3076) touches Lite.Tests/CrossAppGuardCiGateTests.cs — the same file, the C# string-literal stage. Merged at df5f0467c, and this one did conflict, in two hunks. Both resolved to keep both lanes' work: their two new pins (TheCSharpMatchers_StillCannotSeeTheseSpellings, TheBareDirectoryArm_IsLiveForBothSkus) and their two new CSharpPaths doc paragraphs, alongside my MSBuild pins — while dropping the four evaluator pins this PR deletes, which their side reintroduced simply by being present on dev.

A clean merge is the case to distrust, and the second merge is why. #3079 adds a bare-directory arm to CSharpPaths, which changes the found set: two new references (Lite (directory), Lite.Tests (directory)) and a new DarlingTestsBackstopped entry for one of them. Nothing textual would have told me that; only re-running the assertions did.

Everything re-derived at df5f0467c:

figure	before either merge	at df5f0467c
found-set diff, shipped Scan, dev's file vs mine	41 / 41, comm empty both ways	43 / 43, comm empty both ways
references per arm	22 Lite / 19 Darling	22 / 21 (#3079's bare-directory arm adds two on the Darling arm)
C# files opened per arm	299 / 454	300 / 457
-getItem elapsed	0.30 s / 0.34 s	0.30 s / 0.32 s
<Import> over tracked MSBuild files	0 of 24	0 of 24
CS1574 in the two changed files	0	0
Lite suite, harness	10 pass	12 pass, 0 failed, 0 not run — my 10 plus #3079's 2
DocCommentHygieneTests + CommentFilterAdoptionTests, harness	42 pass	44 pass, 0 failed, 0 not run
full Lite.Tests rebuild, macOS	0 errors	0 errors, 9 pre-existing warnings, none in either changed file

The reference total moving 41 → 43 is attributable rather than mysterious: it is exactly #3079's two bare-directory references, and the superset diff is empty at both bases. The 10 → 12 and 42 → 44 suite deltas are #3079's and #3077's new pins, not mine — which is why the load-bearing evidence here is Failed: 0 plus Not Run: 0 plus an empty set difference, all per-run facts, rather than a total.

The MCP census moved as expected, and did not matter: 87 Lite / 138 Darling at the original base, 87 / 139 after #2880. Re-derived at each head by running the shipped ExtractToolNames through reflection, not by grepping. Nothing here depends on either figure.

Review findings

Four findings across three review threads, all from the review bot, all correct, all fixed, all threads resolved:

1. using System.Security; was unused — left from an earlier fixture that interpolated the repository's absolute path into the generated XML and needed escaping. The fixture writes relative MSBuild properties instead. 8b5122b85.

2. The kill-after-timeout catch took InvalidOperationException alone. Right, and it understates the gap: Kill(entireProcessTree: true) documents four — InvalidOperationException, NotSupportedException, Win32Exception, and AggregateException for a descendant that could not be killed, which the process-tree overload adds for itself. All four are caught now; the first three match DarlingDeployRollbackRetentionTests and DarlingDeployStaleFileTests, and the comment says why this site has a fourth. The same thread caught a comment claiming "the exit code below is still read" when the branch returns -1 — gone, and the message now says no output was collected rather than leaving an empty stdout to be read as an answer. 63b41adf9.

   That finding also surfaced the one floor of four I had not pinned. The deadline could not be tested because the shipped two minutes cannot be waited out, so RunDotnet now takes it as a parameter defaulting to the shipped value, and mutations 15 and 16 below cover it.

3. ParseEvaluation read properties with an unguarded GetString() while the sibling item-metadata reader checks ValueKind first. GetString() raises on a non-string rather than returning null, and an exception out of that method bypasses the Evaluation.Failure floor every caller asserts on — which is its only contract. Not unreachable either: the loud-failure pin calls it directly with hand-written JSON. Fixed in b47d4d8ba, and reported rather than coerced to the empty string the way item metadata is, because empty is MSBuild's own answer for "there is no Directory.Build.props" and flattening an unexpected shape into one would manufacture that answer. Pinned, plus one more the same reasoning exposed: an answer missing a requested property, which would otherwise leave the build-file paths reading as "there are none". Mutation 17.

4. A fourth finding came from the review's own second pass and is worth its own note: CSharpPaths' summary still carried <see cref="MsBuildPaths"/>, a method this change deletes. Real rot, and nothing in the build catches it — GenerateDocumentationFile is off, so the compiler never binds a cref. Fixed in 330f6958f. Rather than patch the reported line, I built the project once with -p:GenerateDocumentationFile=true and let the compiler enumerate every one: 22 distinct CS1574 across the Lite.Tests compilation, none in either file this PR touches. (Roughly half of the 22 look genuinely stale — AppliesTo five times, PayloadColumns, IsVictim, WaitTime — and the rest name types in assemblies Lite does not reference, which a cref cannot resolve by design. Pre-existing either way; the split needs triage rather than a number.)

   A stale comment in Resolve describing the deleted wildcard reduction was found the same way and fixed in the merge commit.

[claude]

Reviewed. This is test-infra-only (Lite.Tests/CrossAppGuardCiGateTests.cs + a doc-comment tweak in CrossAppMcpToolInventoryPinTests.cs), no production Lite/Darling code touched, so no Lite/Darling parity drift to flag — the guard intentionally lives only in Lite.Tests and scans both trees from there, which matches the existing pattern.

Went through the shell-out path specifically for the things that matter for a CI gate like this:

• No injection risk: dotnet args go through ProcessStartInfo.ArgumentList, not a concatenated command-line string, so projectPath and the semicolon-joined item/property lists can't be reinterpreted by a shell.
• Failure handling is genuinely defense-in-depth: process-never-started (Win32Exception), non-zero exit, timeout+Kill(entireProcessTree: true), unparseable stdout, and the "recognised-but-empty-item-type" MSBuild quirk are all floored separately and asserted from both the pins and the shared Check()/AssertInvocationSucceeded path, so they can't drift apart. The stdout/stderr reads are started before WaitForExit, so there's no read-after-exit deadlock on the large (~0.5MB) item payloads mentioned in the PR description.
• HintPath resolution now resolves against DefiningProjectFullPath's directory rather than the consuming project's directory — correct per MSBuild's own resolution rules for metadata declared in an imported file, and consistent with the "attribute to the file that actually declared it" design elsewhere in the file.
• Only found one nit, left inline: an unused using System.Security; was added and nothing in the file references that namespace.

No correctness, security, or performance concerns beyond that nit. Good test coverage of the failure modes that would matter most for a CI gate (silent-empty-answer scenarios).

[claude]

Reviewed. This is a C#-only test-infrastructure change (no T-SQL, no product code), and the single touched class (CrossAppGuardCiGateTests) already intentionally covers both Lite and Darling by design — so there's no Lite/Darling parity drift here, and the CrossAppMcpToolInventoryPinTests.cs edit is a comment-only fix to drop a stale numeral, consistent with #3072's policy.

I traced through the new MSBuild-evaluation logic (Evaluate/ParseEvaluation/CrossAppItems/CrossAppPath), the synthetic fixture in WriteCrossAppFixture, and the resolution-base fix for HintPath (consuming-project directory vs. declaring-file directory) against the PR description's own measurements — the fixture's construction (Imported.props sitting above the consuming project) and the assertions in TheEvaluatedRead_SurvivesEveryFormThatDefeatsAnXmlParse are internally consistent with what's claimed. The dotnet shell-out (RunDotnet) uses ArgumentList rather than a shell-interpreted command line, so there's no injection concern, and both async stream reads start before WaitForExit, avoiding the classic redirect-stream deadlock.

One real (if minor) defect: Lite.Tests/CrossAppGuardCiGateTests.cs:981 — the doc comment on CSharpPaths still reads:

/// Shared by the walk and by the pin that reads it back, for the same reason
/// <see cref="MsBuildPaths"/> is: ...

MsBuildPaths and ReadMsBuildPaths are the old regex-based methods this PR deletes elsewhere in the same file (they backed the previous XML-parsing route). The doc line itself is untouched context, which is why it doesn't show up in the diff, but the method it points at no longer exists post-PR, so it's now a dangling cross-reference. Lite.Tests doesn't set GenerateDocumentationFile, so this won't surface as CS1574 at build time — but a reader following the link hits nothing. Worth repointing (likely at ParsedProjectXmlPaths, which is the actual "pin that reads it back" today) or rewording.

Didn't flag anything else — no missing-index-style DMV concerns apply here (no T-SQL touched), no secrets/network/file-write boundary issues beyond the already-reviewed shell-out, and I didn't spot a performance regression worth calling out given the PR's own measured ~0.3s/project evaluation cost and the deliberate tradeoff discussion in the description.

[claude]

Reviewed. This is C# test infrastructure only (Lite.Tests/CrossAppGuardCiGateTests.cs + a one-line doc update in CrossAppMcpToolInventoryPinTests.cs) — no T-SQL, no app-code changes, so the T-SQL style rules don't apply here and there's no Lite/Darling parity gap to flag: this guard has always lived solely in Lite.Tests by design, scanning both trees from one file.

The replacement of the hand-rolled MSBuild XML/regex parser with dotnet msbuild -getItem -getProperty is well-reasoned and thoroughly tested — the synthetic two-SKU fixture exercises properties, Condition, Choose/When, imported-props-defined items, transforms, and wildcards, and the failure-mode tests correctly pin the "exit 0 + empty array" trap for an unrecognized item type, which is the sharpest edge case here. The process invocation uses ArgumentList throughout (no shell, no injection surface), and the concurrent-read-before-WaitForExit pattern correctly avoids the classic redirected-output deadlock.

One inline finding: the timeout/kill path in RunDotnet only catches InvalidOperationException around process.Kill(entireProcessTree: true), whereas the established pattern elsewhere in this repo (Darling.Tests) also catches NotSupportedException and Win32Exception for the same race. Left as an inline comment with specifics.

No missing-index DMV concerns (none present), no secrets/credentials, no network calls introduced.

[claude]

Reviewed the -getItem/-getProperty rewrite of the project-file scan stage in CrossAppGuardCiGateTests.cs. This is test-only infrastructure (no product/T-SQL code, no Lite/Darling parity concern — these cross-app guard tests intentionally live only under Lite.Tests and scan both trees from one place, and the second changed file (CrossAppMcpToolInventoryPinTests.cs) is doc-comment-only).

Overall this is a solid, carefully-verified rewrite: the process shelling-out avoids shell injection (uses ArgumentList, not string-built commands), the deadlock-avoidance pattern for reading stdout/stderr is correct, the timeout/kill-on-hang path is sound, and the per-item-type floor against a synthetic fixture (rather than the real tree) is a good answer to "an unrecognised item type returns an empty array with exit 0."

One inline finding: in ParseEvaluation, the Properties loop calls property.Value.GetString() without checking ValueKind first (line 1541), while the sibling Metadata() helper used for items guards against exactly this. It won't fire for the properties requested today (MSBuild always emits those as plain strings) but it's inconsistent with the file's explicit "never throw, always report as Evaluation.Failure" design for malformed process output — worth tightening for consistency, low severity.

No security, correctness, or performance issues beyond that.

[claude]

Reviewed the full diff (both files) against CONTRIBUTING.md. This only touches Lite.Tests/CrossAppGuardCiGateTests.cs and Lite.Tests/CrossAppMcpToolInventoryPinTests.cs — pure C# test infrastructure, no T-SQL, so the T-SQL style rules don't apply here.

Parity: No Darling-side counterpart exists for this guard (it's a single Lite.Tests file that scans both Lite and Darling/Darling.Tests trees), and grepping the repo confirms there's no duplicated logic under Darling/ that would need a matching update. No parity drift.

Correctness: Traced the Evaluate → ParseEvaluation → CrossAppItems → CrossAppPath pipeline end to end against the fixture in WriteCrossAppFixture and the assertions in TheEvaluatedRead_SurvivesEveryFormThatDefeatsAnXmlParse. One thing I went in suspicious of: EvaluatedItemTypes is a hardcoded 6-name list (Compile;None;Content;EmbeddedResource;Reference;ProjectReference) passed to -getItem:, so a future cross-app reference expressed through a type outside that list (e.g. a WPF Page/Resource item) wouldn't be requested from MSBuild at all. That looked like a silent-miss risk in the same family as #3063/#3067/#3076. But it's already closed: TheEvaluatedSet_ContainsEverythingAParseOfTheSameXmlFinds runs ParsedProjectXmlPaths — a type-agnostic crude text scan — against the real tree and asserts the typed, evaluated set is a superset of it. A reference living in an unrequested item type would show up in the crude parse but not the evaluated set, and that test would fail loudly. So the design is sound.

Also checked the process-handling in RunDotnet (concurrent stream draining before WaitForExit, the four-exception catch around Kill(entireProcessTree: true), the deadline-as-parameter for testability) — matches what's described in the PR body and looks correct. No command-injection surface (ArgumentList, no shell), no secrets, no unbounded external input.

Performance: Each evaluation shells out to dotnet msbuild (~0.3s/project per the PR's own measurements); this is a real but disclosed and justified tradeoff for correctness, not a hidden regression.

No blocking findings. This is a solid, well-verified swap from a hand-rolled XML regex evaluator to real MSBuild evaluation, with a sensible superset/second-opinion check guarding against the class of gap the switch could otherwise introduce.