Repository navigation
feat: VS-MSBuild restore fallback when the CLI SDK set cannot target the repo - #340
Merged
Merged
Conversation
…the repo
When dotnet restore fails on SDK resolution (NETSDK1045-class: CLI SDKs
older than the repo TFM, or global.json unsatisfiable) scip-csharp now
retries the solution restore once with the newest VS MSBuild located via
vswhere - VS ships its own MSBuild + SDK targets independent of
C:\Program Files\dotnet, so it restores what the CLI cannot even
evaluate. SDK-resolution detection reads stdout AND stderr: NETSDK1045
prints to stdout on current dotnet hosts (verified live with a net99.0
probe). Bounded wait with the existing pending-marker/gated-exit
contract; on failure the per-project fallback still runs (mixed-TFM
recovery preserved).
The MSBuild instance choice for the workspace load is now
capability-aware too: when every CLI SDK instance is older than the
repo's raw-text TFM floor (max over all TFMs + global.json major), the
highest VS instance is registered instead - the SDK preference stands
everywhere else, including its net472-BuildHost-crash rationale.
Motivated by the 2026-10-15 HUSQ worktree incident (hollow 10.0.101 SDK
dir left the CLI blind to net10 while VS built fine).
Deliberate behavior change: SDK-instance pick is the HIGHEST qualifying
SDK (was first-enumerated, an undocumented order); single-SDK machines
unaffected.
Review-fixes:
- [Important] NETSDK1045/global.json detection read stderr only while the
trigger text prints to stdout on current hosts -> classify over both
streams; match current ('compatible .NET SDK') and legacy
('compatible installed .NET SDK') global.json wordings.
- [Important] multi-target floor took the FIRST TFM (net8.0;net10.0
floored at 8) -> take the max over all TFMs.
- [Important] inline 10_000 vswhere wait -> VsWhereWaitMs const.
- [Important] healthy-machine invariant weakened on multi-SDK machines ->
kept highest-qualifying (matches the original 'highest available
MSBuild' intent) and recorded the change in the branch plan.
17 tests over the fallback's pure logic: SDK-resolution detection pinned against the live host wordings (NETSDK1045 on stdout, current and legacy global.json phrasings; NU11xx feed/auth failures must NOT trigger), vswhere -find parsing, the capability-aware instance choice (qualifying SDK beats newer VS, floor-miss falls to VS, no-VS keeps SDK as last resort, highest sibling wins), and the repo TFM floor scan (multi-target max, project/global.json composition, parent-dir walk-up, null paths). The multi-target pin was defect-probed: first-TFM-only fails the floor assert as predicted. Also fixes the vswhere probe bound (round-1 review note): async drain before the bounded WaitForExit, and a KILL on timeout — vswhere is a local metadata probe, not a restore, so its death costs one failed probe; the kill also closes the pipes so the stdout read cannot block. Review-fixes: - [Important] WaitForExit timeout result was discarded and the stdout GetResult still blocked until process exit, so VsWhereWaitMs never actually bounded a hung vswhere -> kill(entireProcessTree) on timeout and return null.
CHANGELOG entry under the pending 1.5.31 heading, highlights line in AGENTS.develop.md, and the branch registered in the active-branches table. Plan stage 3 ticked in the branch-local AGENTS.md. Validation: dotnet build 0w/0e; dotnet test 99/99; cargo fmt --check clean; cargo clippy --all-targets -D warnings clean; cargo test --lib --bins all green (Rust side untouched by this branch).
The floor regex ^net(\d+)(?:\.|$) matched the extension-less .NET Framework spellings (net48 -> 48, net472 -> 472), so a solution with one net4x project floored at 48: no CLI SDK qualifies, and healthy SDK+VS machines flipped to the VS MSBuild instance for no reason (exposing the documented net472-BuildHost crash risk where the SDK path loaded those projects fine). The minor dot is now required (^net(\d+)\.) - every net-core TFM has one, no net4x spelling does. Pinned by NetFrameworkSpellingsDoNotRaiseTheFloor (net48 alone -> null, net48;net10.0 -> 10); defect-probed: the old regex fails the mixed case with a 48 floor. Final-review fix (round 1 finding). Round-2 re-review executed by the worker itself (reviewer subagent hit its 5h usage limit); scope was the fix diff only - verified: regex at HEAD, diff touches solely MsbuildRestore.cs + tests, 100/100 green, round-1 met-rows untouched.
flupkede
pushed a commit
that referenced
this pull request
Oct 16, 2026
flupkede
added a commit
to belovaf/codesearch
that referenced
this pull request
Oct 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Field incident 2026-10-15 (HUSQ worktree): a hollow
10.0.101SDK dir inC:\Program Files\dotnet\sdkleft the dotnet CLI unable to target net10 —dotnet restorefailed NETSDK1045 on every project (0/16 recovered), projects had no existing assets, and the workspace loaded 0 projects, so the symbol index died while VS itself built fine on the same machine (VS ships its own MSBuild + SDK targets, blind spots of the CLI included).What
dotnet restorefails on SDK resolution (NETSDK1045 — detection reads stdout AND stderr, since NETSDK1045 prints to stdout on current hosts — or global.json unsatisfiable, both current and legacy wordings), scip-csharp retries the solution restore ONCE with the newest VS MSBuild found via vswhere. Bounded wait with the existing pending-marker/gated-exit-75 contract (a running restore is never killed); on VS failure the per-project dotnet fallback still runs (mixed-TFM recovery preserved).TargetFrameworkmajors in the solution's csproj files + the nearestglobal.jsonmajor walking up (NuGet's own order; net4x spellings and netstandard/netcoreapp deliberately never raise it). When no CLI SDK instance qualifies and a VS instance exists, the VS instance is registered (explicit log) — otherwise the SDK preference stands, including its net472-BuildHost-crash rationale. Single-SDK machines are byte-identical; multi-SDK machines now deterministically take the highest qualifying SDK (was first-enumerated, an undocumented order — deliberate, disclosed).Validation