Skip to content

Add MSBuild quality review agentic workflow#19958

Merged
T-Gro merged 1 commit into
dotnet:mainfrom
0101:msbuild-quality-review
Jun 17, 2026
Merged

Add MSBuild quality review agentic workflow#19958
T-Gro merged 1 commit into
dotnet:mainfrom
0101:msbuild-quality-review

Conversation

@0101

@0101 0101 commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Problem

The F# SDK build logic shipped in the .NET SDK and the FSharp.Build assembly — Microsoft.FSharp.NetSdk.targets, Microsoft.FSharp.Targets, the vsintegration/shims/*, etc. — is imported by every F# project, yet nothing systematically audits these .props/.targets files for MSBuild authoring anti-patterns: *DependsOn chain overwrites, unquoted conditions, missing Exists() import guards, semicolon-list clobbering, non-portable paths. A regression in one of these files silently affects all F# builds.

Changes

Adds a new GitHub Agentic Workflow (gh aw) that runs weekly (and on demand), audits the repo's MSBuild files, and files an issue with categorized findings; it can also open a draft PR for safe, low-risk fixes.

  • .github/workflows/msbuild-quality-review.md — workflow source (frontmatter + reviewer prompt).
  • .github/workflows/msbuild-quality-review.lock.yml — generated by gh aw compile (not hand-edited).

Adapted from microsoft/testfx's MSBuild-quality-review workflow, retuned for this repo:

  • Discovery rewritten for the F# layout — case-insensitive find (-iname) so the capital-.Targets crown jewels (Microsoft.FSharp.Targets, Microsoft.Portable.FSharp.Targets) aren't missed; prioritizes shipped SDK build logic in src/FSharp.Build/, src/fsc/, src/fsi/, vsintegration/shims/; excludes the downloaded .dotnet/ SDK plus obj/bin/artifacts/packages.
  • Down-weighted the NuGet build/ rules that don't apply here — F# ships build logic via the .NET SDK / FSharp.Build, not NuGet build//buildTransitive/ folders — and emphasized the target-authoring, property, item, and import rules that are relevant.
  • House-style frontmatter matching the repo's existing workflows: default copilot engine, network: [defaults, dotnet], read-only permissions with writes only via safe-outputs. Labels automation + Area-ProjectsAndBuild (both already exist).
  • Removed the testfx-specific shared/reporting.md import (absent here) and scoped Phase 6 build validation so it can't exceed the 30-minute timeout.

Tests

  • gh aw compile (v0.76.1, matching the repo's other lock files) — clean, 0 errors.
  • End-to-end trial run (gh aw trial on real GitHub Actions): activation → agent → detection → safe_outputs → conclusion all green; produced the expected [msbuild-quality] … issue with correct labels. Spot-checked findings were accurate against source, e.g.:
    • Microsoft.FSharp.Targets:224CoreCompileDependsOn set without $(CoreCompileDependsOn); (drops prior targets).
    • Microsoft.FSharp.NetSdk.targets:174 — unquoted condition operands.

Notes for reviewers

  • The .lock.yml is machine-generated; review the .md and recompile rather than editing the lock.
  • Labels use existing repo labels; engine left as the repo default (copilot) — easy to adjust.
  • Triggers only fire once the lock file is on the default branch.

Port and adapt microsoft/testfx PR dotnet#8365 ("MSBuild quality review") to a
gh-aw workflow for dotnet/fsharp. Runs weekly, audits the repo's .props /
.targets / .Targets files for MSBuild authoring issues, files an issue with
findings, and can open a draft PR for safe, low-risk fixes.

F#-specific adaptations:
- Phase 1 discovery rewritten for the fsharp layout: case-insensitive search
  (-iname) catches the capital-.Targets crown jewels (Microsoft.FSharp.Targets,
  Microsoft.Portable.FSharp.Targets); prioritizes the shipped F# SDK build
  logic in src/FSharp.Build, src/fsc, src/fsi and the vsintegration/shims;
  excludes .dotnet/ (the downloaded SDK), obj/bin/artifacts/packages.
- Removed the nonexistent shared/reporting.md import; report template is inline.
- Down-weighted the NuGet build/ rules (D-2 and all of E) that don't apply here
  (fsharp ships via the .NET SDK / FSharp.Build, not NuGet build/ folders) and
  emphasized Categories A, B, C, D-1/3/4/5.
- House-style frontmatter: default copilot engine, network [defaults, dotnet],
  read-only permissions with safe-outputs for writes. Labels automation +
  Area-ProjectsAndBuild (both already exist in dotnet/fsharp).
- Phase 6 build validation scoped (no full ./build.sh) to fit the 30-min timeout.

Lock file compiled with gh aw v0.76.1, matching the repo's existing workflows.
@github-actions

Copy link
Copy Markdown
Contributor

✅ No release notes required

@github-actions github-actions Bot added ⚠️ Affects-Agent-Config Tooling check: PR modifies AI agent instructions or workflows ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure labels Jun 16, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Agent-Config, Affects-Build-Infra
Affects-Agent-Config: adds agentic workflow definition (.md + .lock.yml)
Affects-Build-Infra: new CI workflow that executes on schedule/dispatch

Generated by PR Tooling Safety Check · opus46 5.9M ·

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Jun 17, 2026
@T-Gro T-Gro merged commit 35851a1 into dotnet:main Jun 17, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Agent-Config Tooling check: PR modifies AI agent instructions or workflows ⚠️ Affects-Build-Infra Tooling check: PR touches build infrastructure

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants