Add shift-left header annotations to the C# toolchain

.github/copilot-instructions.md

@@ -22,19 +22,25 @@ dotnet build generation/WinSDK -c Release -p:ScanArch=crossarch -t:ScrapeHeaders
 ## Test Commands
 
 ```powershell
-# Run all tests
+# Run all tests after generating the WinMD (requires Python 3)
 ./scripts/DoTests.ps1
 
 # Individual test suites
 dotnet test tests/MetadataUtils.Tests -c Release
 dotnet test tests/Windows.Win32.Tests -c Release          # winmd integrity tests
 dotnet test tests/Win32MetadataScraperTests -c Release     # scraper heuristic tests
-dotnet test tests/ClangSharpSourceToWinmdTests -c Release
+dotnet test tests/ClangSharpSourceToWinmdTests -c Release   # emitter and cross-architecture composition
+python -m unittest discover -s tests\SdkAnnotationCoverage.Tests -p "test_*.py"
 
 # Single test
 dotnet test tests/Win32MetadataScraperTests -c Release --filter "TransitiveInclude"
 ```
 
+`DoTests.ps1` runs the utility, emitter, SDK annotation coverage-auditor, scraper,
+and WinMD integrity suites. GitHub Actions and Azure Pipelines invoke this same
+runner after generating the WinMD. The Python suite uses the standard library
+and tests the annotation coverage auditor, not the separate Node workflow tests.
+
 ## Architecture
 
 The toolchain converts Windows SDK C/C++ headers into a .NET metadata assembly (`Windows.Win32.winmd`).
@@ -66,6 +72,10 @@ Each arch job scrapes ALL partitions for that architecture.
 
 **NuGet feed:** The repo uses a private ADO Artifact Feed (`Win32Metadata-Dependencies`) as its sole package source. New packages from nuget.org must be saved to this feed first.
 
+**Import discovery before header overrides:** Before adding `_Win32_ImportLibrary_` or a manual DLL mapping, inspect the selected SDK's import libraries and the library-processing set in `scripts\CreateProcLibMappingForAllLibs.ps1`. Use `WinmdUtils dumpImports --lib <path>` to verify the actual symbols and DLL, not just the library filename. If a matching `.lib` was omitted or its imports were missed, fix shared discovery instead of adding a redundant header annotation. Use an authored override only when no authoritative SDK library mapping is available or the native contract genuinely requires an override; record the SDK/library search evidence.
+
+**Header-annotation migration:** Follow `docs\copilot\plans\header-annotation-migration-playbook.md`. Classify the original artifact before editing: patch authored `.h` files directly, but patch original `.idl` and regenerate all affected outputs for IDL-owned declarations. Never patch a generated `.h` or independently patch the same shared artifact in different partitions. Use exact qualified tooling/pilot pins and keep raw OLD-to-NEW differences visible.
+
 **Winmd equivalence:** After any change, the winmd must be compared against the baseline. The `NoSuggestedRemappings` test in `Windows.Win32.Tests` validates that no new unhandled remappings were introduced. API diffs are automatically posted as PR comments by CI. Run `.\scripts\DiffWinmdToBaseline.ps1` locally to see a full C# declaration diff against the last release.
 
 **Cross-partition remaps:** Remaps in `scraper.settings.rsp` are global — they apply to all partitions. Auto-discovered remaps (from `Win32MetadataScraper`) are per-partition. If a tag name is used across partitions but the typedef is only in one partition's headers, the remap must be global (in `scraper.settings.rsp`) OR the referencing partition must `#include` the header with the typedef.

.github/tests/pr-api-diff-comment.test.cjs

@@ -1,16 +1,32 @@
 const assert = require('node:assert/strict');
-const { readFileSync } = require('node:fs');
+const { execFileSync } = require('node:child_process');
+const { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } = require('node:fs');
+const { tmpdir } = require('node:os');
 const path = require('node:path');
 const { test } = require('node:test');
 
 const workflow = readFileSync(path.join(__dirname, '..', 'workflows', 'pr-api-diff-comment.yml'), 'utf8')
   .replace(/\r\n/g, '\n');
-const steps = new Map(workflow.split(/^      - name: /m).slice(1).map(block => {
-  const endOfName = block.indexOf('\n');
-  return [block.slice(0, endOfName), block.slice(endOfName + 1)];
-}));
+const validationWorkflow = readFileSync(path.join(__dirname, '..', 'workflows', 'pr-validation.yml'), 'utf8')
+  .replace(/\r\n/g, '\n');
+const steps = workflowSteps(workflow);
+const validationSteps = workflowSteps(validationWorkflow);
 const AsyncFunction = Object.getPrototypeOf(async function () {}).constructor;
 
+function workflowSteps(source) {
+  return new Map(source.split(/^      - name: /m).slice(1).map(block => {
+    const endOfName = block.indexOf('\n');
+    return [block.slice(0, endOfName), block.slice(endOfName + 1)];
+  }));
+}
+
+function validationJob(name) {
+  const marker = `  ${name}:\n`;
+  const start = validationWorkflow.indexOf(marker);
+  assert.notEqual(start, -1, `Missing validation job: ${name}`);
+  return validationWorkflow.slice(start + marker.length).split(/^  [\w-]+:\s*$/m)[0];
+}
+
 function script(name) {
   const step = steps.get(name);
   assert.ok(step, `Missing workflow step: ${name}`);
@@ -23,6 +39,14 @@ function script(name) {
 
 const checkArtifact = script('Check API diff artifact');
 const postComment = script('Post API diff comment');
+const oversizedPatch = [
+  'diff --git a/bin/baseline.apidump.cs b/bin/current.apidump.cs',
+  '--- a/bin/baseline.apidump.cs',
+  '+++ b/bin/current.apidump.cs',
+  '@@ -1,4000 +1,8000 @@',
+  ...Array.from({ length: 4000 }, (_, index) => `-public const int OldValue${index} = ${index};`),
+  ...Array.from({ length: 8000 }, (_, index) => `+public const int NewValue${index} = ${index};`)
+].join('\r\n') + '\r\n';
 
 function fixture(documentationOnly = false) {
   return {
@@ -145,6 +169,112 @@ test('download and comment both require the preflight output', () => {
   assert.doesNotMatch(workflow, /uses: actions\/checkout/);
 });
 
+test('both artifact uploads still include the generated patch', () => {
+  const upload = validationSteps.get('Upload API diff');
+  assert.match(upload, /^          name: api-diff$/m);
+  assert.match(upload, /^          path: bin\/api-diff\.patch$/m);
+  const winmdUpload = validationSteps.get('Upload winmd and API dump');
+  assert.match(winmdUpload, /^          name: winmd$/m);
+  for (const file of ['Windows.Win32.winmd', 'current.apidump.cs', 'baseline.apidump.cs', 'api-diff.patch']) {
+    assert.ok(winmdUpload.includes(`            bin/${file}\n`));
+  }
+});
+
+for (const [jobName, uploadCount] of [['scrape', 1], ['build-test', 2]]) {
+  test(`${jobName} always-run local uploads require a successful checkout`, () => {
+    const job = validationJob(jobName);
+    assert.match(job, /^      - uses: actions\/checkout@[^\n]+\n        id: checkout$/m);
+    const uploads = [...workflowSteps(job).values()].filter(step =>
+      /^        uses: \.\/\.github\/actions\/upload-artifact-with-retry$/m.test(step) &&
+      /^        if: always\(\)/m.test(step));
+    assert.equal(uploads.length, uploadCount);
+    for (const upload of uploads) {
+      const condition = upload.match(/^        if: (.+)$/m)[1];
+      const shouldUpload = new Function('needs', 'steps', 'always', `return (${condition});`);
+      const needs = { changes: { outputs: { build_needed: 'true' } } };
+      for (const outcome of ['', 'skipped', 'failure', 'cancelled']) {
+        assert.equal(shouldUpload(needs, { checkout: { outcome } }, () => true), false,
+          `Local action must not load after checkout outcome ${JSON.stringify(outcome)}.`);
+      }
+      assert.equal(shouldUpload(needs, { checkout: { outcome: 'success' } }, () => true), true,
+        'Diagnostics must still upload after later build or test failures.');
+      if (jobName === 'build-test') {
+        needs.changes.outputs.build_needed = 'false';
+        assert.equal(shouldUpload(needs, { checkout: { outcome: 'success' } }, () => true), false);
+      }
+    }
+  });
+}
+
+test('failed prerequisite scrapes still fail before checkout without masking the primary error', () => {
+  const job = validationJob('build-test');
+  const verify = workflowSteps(job).get('Verify scrape jobs');
+  const condition = verify.match(/^        if: (.+)$/m)[1];
+  const shouldFail = new Function('needs', `return (${condition});`);
+  for (const result of ['failure', 'cancelled', 'skipped']) {
+    assert.equal(shouldFail({
+      changes: { outputs: { build_needed: 'true' } }, scrape: { result }
+    }), true);
+  }
+  assert.equal(shouldFail({
+    changes: { outputs: { build_needed: 'true' } }, scrape: { result: 'success' }
+  }), false);
+  assert.equal(shouldFail({
+    changes: { outputs: { build_needed: 'false' } }, scrape: { result: 'skipped' }
+  }), false);
+  assert.ok(job.indexOf('      - name: Verify scrape jobs\n') <
+    job.indexOf('      - uses: actions/checkout@'));
+  assert.doesNotMatch(verify, /continue-on-error: true/);
+  const command = verify.match(/^        run: (.+)$/m)[1];
+  assert.throws(() => execFileSync('pwsh',
+    ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', command], { windowsHide: true }),
+  error => error.status !== 0 &&
+    /One or more header scrape jobs did not succeed\./.test(error.stderr.toString()));
+});
+
+for (const [name, patch, expectedOutputs] of [
+  ['an oversized diff', oversizedPatch, { has_diff: 'true', additions: '8000', deletions: '4000' }],
+  ['an empty diff', '', { has_diff: 'false' }]
+]) {
+  test(`the producer preserves ${name} byte-for-byte`, () => {
+    const directory = mkdtempSync(path.join(tmpdir(), 'win32metadata-api-diff-'));
+    try {
+      const baselineDirectory = path.join(directory, 'bin', 'baseline-artifact');
+      mkdirSync(baselineDirectory, { recursive: true });
+      writeFileSync(path.join(baselineDirectory, 'Windows.Win32.winmd'), '');
+      writeFileSync(path.join(directory, 'expected.patch'), patch);
+      const outputFile = path.join(directory, 'step-output.txt');
+      const step = validationSteps.get('Generate API surface diff');
+      const marker = '        run: |\n';
+      const start = step.indexOf(marker);
+      assert.notEqual(start, -1, 'Missing inline diff producer');
+      const producer = step.slice(start + marker.length).replace(/^ {10}/gm, '');
+      const commands = String.raw`
+        function dotnet { $global:LASTEXITCODE = 0 }
+        function cmd {
+          if ($args.Count -ne 2 -or $args[0] -ne '/c' -or
+              $args[1] -ne 'git diff --no-index --unified=3 bin\baseline.apidump.cs bin\current.apidump.cs > bin\api-diff.patch 2>&1') {
+            throw 'Unexpected diff command'
+          }
+          Copy-Item -LiteralPath expected.patch -Destination bin\api-diff.patch
+          $global:LASTEXITCODE = ${patch.length === 0 ? 0 : 1}
+        }
+      `;
+      execFileSync('pwsh', ['-NoLogo', '-NoProfile', '-NonInteractive', '-Command', commands + producer], {
+        cwd: directory,
+        env: { ...process.env, GITHUB_OUTPUT: outputFile },
+        windowsHide: true
+      });
+      const actual = readFileSync(path.join(directory, 'bin', 'api-diff.patch'));
+      assert.ok(actual.equals(Buffer.from(patch)), `Patch changed: expected ${Buffer.byteLength(patch)} bytes, got ${actual.length}`);
+      const outputs = Object.fromEntries(readFileSync(outputFile, 'utf8').trim().split(/\r?\n/).map(line => line.split('=')));
+      assert.deepEqual(outputs, expectedOutputs);
+    } finally {
+      rmSync(directory, { recursive: true, force: true });
+    }
+  });
+}
+
 test('recorded documentation-only execution skips the download and comment', async () => {
   const result = await runWorkflow(fixture(true));
   assert.deepEqual(result.outputs, { available: 'false' });
@@ -162,6 +292,46 @@ test('a current build artifact downloads and posts the actual diff', async () =>
   assert.equal(result.comments[0].issue_number, 42);
   assert.match(result.comments[0].body, /\+1 additions \/ -1 deletions/);
   assert.match(result.comments[0].body, /\+added\n-removed/);
+  assert.doesNotMatch(result.comments[0].body, /Diff truncated/);
+});
+
+test('an oversized diff retains full statistics and links to the complete artifact', async () => {
+  const data = fixture();
+  data.patch = oversizedPatch;
+  const result = await runWorkflow(data);
+  const body = result.comments[0].body;
+  assert.match(body, /\+8000 additions \/ -4000 deletions/);
+  assert.ok(body.includes('Diff truncated'));
+  assert.ok(body.includes(`[api-diff artifact](${data.run.html_url}#artifacts)`));
+  assert.ok(body.length <= 65536, `Comment is ${body.length} characters`);
+  assert.ok(!body.includes('+public const int NewValue7999 = 7999;'));
+  assert.equal(data.patch, oversizedPatch);
+});
+
+for (const [name, patch] of [
+  ['wrapper and truncation notice', '+' + 'x'.repeat(65500)],
+  ['long Markdown fences', '+' + '`'.repeat(34000) + '\n'],
+  ['many backtick runs', '+`value`\n'.repeat(70000)]
+]) {
+  test(`the complete comment fits the limit including ${name}`, async () => {
+    const data = fixture();
+    data.patch = patch;
+    const result = await runWorkflow(data);
+    const body = result.comments[0].body;
+    assert.ok(body.length <= 65536, `Comment is ${body.length} characters`);
+    assert.ok(body.includes('Diff truncated'));
+    assert.ok(body.startsWith('<!-- win32metadata-api-diff -->'));
+    assert.match(body, /<\/details>\n/);
+    assert.ok(body.endsWith('> This comment is automatically updated on each push.'));
+  });
+}
+
+test('backticks in an untruncated diff remain inside its Markdown fence', async () => {
+  const data = fixture();
+  data.patch = '+```embedded fence```\n';
+  const result = await runWorkflow(data);
+  assert.ok(result.comments[0].body.includes('````diff\n' + data.patch + '\n````'));
+  assert.doesNotMatch(result.comments[0].body, /Diff truncated/);
 });
 
 test('an empty actual patch still posts no API differences', async () => {

.github/workflows/pr-api-diff-comment.yml

@@ -159,10 +159,16 @@ jobs:
               const diffLines = diff.split(/\r?\n/);
               const additions = diffLines.filter(line => /^\+[^+]/.test(line)).length;
               const deletions = diffLines.filter(line => /^-[^-]/.test(line)).length;
-              const longestBacktickRun = Math.max(0, ...Array.from(diff.matchAll(/`+/g), match => match[0].length));
-              const fence = '`'.repeat(Math.max(3, longestBacktickRun + 1));
-
-              body = `${marker}
+              const maxCommentLength = 65536;
+              const truncationNotice = `\n\n> Diff truncated to fit this comment. Download the complete \`api-diff.patch\` from the [api-diff artifact](${runUrl}#artifacts).`;
+              const renderComment = (preview, notice = '') => {
+                let longestBacktickRun = 0;
+                for (const match of preview.matchAll(/`+/g)) {
+                  longestBacktickRun = Math.max(longestBacktickRun, match[0].length);
+                }
+                const fence = '`'.repeat(Math.max(3, longestBacktickRun + 1));
+
+                return `${marker}
             ## 📋 API Surface Diff
 
             **+${additions} additions / -${deletions} deletions** vs main branch ([full validation run](${runUrl}))
@@ -171,12 +177,19 @@ jobs:
             <summary>Click to expand API diff</summary>
 
             ${fence}diff
-            ${diff}
+            ${preview}
             ${fence}
 
-            </details>
+            </details>${notice}
 
             > This comment is automatically updated on each push.`;
+              };
+              body = renderComment(diff);
+              if (body.length > maxCommentLength) {
+                // Reserve space for the wrapper, both fences, and the truncation notice.
+                const previewLength = Math.max(0, maxCommentLength - (body.length - diff.length) - truncationNotice.length);
+                body = renderComment(diff.slice(0, previewLength), truncationNotice);
+              }
             }
 
             const comments = await github.paginate(github.rest.issues.listComments, {

.github/workflows/pr-validation.yml

@@ -117,6 +117,7 @@ jobs:
 
     steps:
       - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+        id: checkout
         with:
           lfs: true
           fetch-depth: 0
@@ -145,7 +146,7 @@ jobs:
 
       - name: Upload build logs
         uses: ./.github/actions/upload-artifact-with-retry
-        if: always()
+        if: always() && steps.checkout.outcome == 'success'
         with:
           name: scrape_logs_${{ matrix.arch }}
           path: bin/logs
@@ -174,6 +175,7 @@ jobs:
         run: throw 'One or more header scrape jobs did not succeed.'
 
       - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+        id: checkout
         if: needs.changes.outputs.build_needed == 'true'
         with:
           lfs: true
@@ -335,14 +337,6 @@ jobs:
             "deletions=$deletions" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
 
             Write-Host "+$additions additions / -$deletions deletions"
-
-            # Truncate if too large for a GH comment
-            $diffText = $diffLines -join "`n"
-            $maxLen = 60000
-            if ($diffText.Length -gt $maxLen) {
-              $diffText = $diffText.Substring(0, $maxLen) + "`n`n... (diff truncated - see full artifact)"
-              Set-Content -Path bin\api-diff.patch -Value $diffText -Encoding UTF8
-            }
           }
 
           # Reset LASTEXITCODE so GitHub Actions doesn't treat this step as failed
@@ -358,7 +352,7 @@ jobs:
 
       - name: Upload winmd and API dump
         uses: ./.github/actions/upload-artifact-with-retry
-        if: always() && needs.changes.outputs.build_needed == 'true'
+        if: always() && needs.changes.outputs.build_needed == 'true' && steps.checkout.outcome == 'success'
         with:
           name: winmd
           path: |
@@ -370,7 +364,7 @@ jobs:
 
       - name: Upload build logs
         uses: ./.github/actions/upload-artifact-with-retry
-        if: always() && needs.changes.outputs.build_needed == 'true'
+        if: always() && needs.changes.outputs.build_needed == 'true' && steps.checkout.outcome == 'success'
         with:
           name: build_logs
           path: bin/logs

BuildTools/BuildTools.proj

@@ -9,8 +9,8 @@
 
   <ItemGroup>
     <!-- These are here to support CommonUtils.ps1, which parses the .props file that nuget restore generates based on these. -->
-    <PackageReference Include="Microsoft.Windows.SDK.CPP" Version="10.0.26100.7705" GeneratePathProperty="true"/>
-    <PackageReference Include="Microsoft.Windows.SDK.CPP.x64" Version="10.0.26100.7705" GeneratePathProperty="true"/>
+    <PackageReference Include="Microsoft.Windows.SDK.CPP" Version="10.0.28000.2705" GeneratePathProperty="true"/>
+    <PackageReference Include="Microsoft.Windows.SDK.CPP.x64" Version="10.0.28000.2705" GeneratePathProperty="true"/>
     <!-- Used by CommonUtils.ps1 to get the path to the last release .winmd -->
     <PackageReference Include="Microsoft.Windows.SDK.Win32Metadata" Version="$(LastWin32MetadataReleaseVersion)" GeneratePathProperty="True">
       <PrivateAssets>all</PrivateAssets>