Publish PR artifacts early and reuse them across validation

[Nikola Metulev (nmetulev)]

Description

Publish downloadable PR packages before full validation finishes, then run validation in parallel without removing any test or merge gate.

• One artifact-producing runner builds both CLI architectures and all packages with -SkipTests -SkipDocs. Existing CLI, npm, MSIX and four NuGet artifacts become downloadable immediately, labeled awaiting validation.
• Independent consumers run two CLI shards, Auxiliary, UIAutomation, docs, UI E2E/coordination and all 14 samples from those same-run artifacts without republishing.
• CLI shard 1 selects PackageCommandTests; shard 2 uses its exact complement, automatically covering new tests. Each invocation checks nonempty/exhaustive discovery, at least one executed test, exact discovery-to-TRX totals including intentional skips, complete result states and coverage output. No process failure is ignored.
• Auxiliary runs Node unit tests, analyzer/stand-down, NuGet Pester and scripts Pester once. Both CLI shards prepare the Node CLI for integration tests. Analyzer compilation retains warnings-as-errors.
• Merge exactly two CLI TRX reports plus UIAutomation into the existing test-results artifact. Reject missing/extra reports and union overlapping coverage rather than averaging percentages or duplicating source-line denominators.
• Preserve npm codegen/format/lint/compile checks, docs, all samples and interactive tests, and the strict top-level build-and-package / test-samples-result checks. Failures, cancellations and unexpected dependency skips cannot pass.
• Reuse same-run artifacts safely on fork PRs; no privileged PR-code execution. Cancel only superseded PR runs. Failed non-PR validation cannot replace the metrics baseline. Sample JUnit paths are workspace-absolute and missing uploads fail.

Default local/ADO All behavior and full main/manual validation remain. Manual sample runs stay self-contained. The parallel-architecture experiment and its unpublished modes/provenance scaffolding were removed with user approval after measuring insufficient benefit.

Review fixes verified on current HEAD

bc0dece8 addresses both issues in the artifact-rerun/reporting review: each producer overwrites only its own uniquely named artifacts on rerun, and failed main/manual validation still publishes combined test reports without promoting a failed metrics baseline.

Run 35384710380 passed on that HEAD, including two controlled partial reruns:

• Attempt 2 reran Auxiliary and its dependent reporting jobs. Only validation-results-Auxiliary, test-results, and metrics-data received new artifact IDs; the other 23 IDs and digests were unchanged.
• Attempt 3 reran the reusable packaging-cli sample and its aggregate checks. Only test-results-packaging-cli was replaced; the other 25 artifacts were unchanged.
• The package producer retained its original start/end times across both attempts: no rebuild, no artifact-name conflicts, and no deleted sibling reports.

All 25 targeted workflow regressions and actionlint passed, including the failed-main report/baseline-promotion truth table. Both architectures built and generated documentation validated locally. Current merged reports contain 7,531 .NET cases: 7,494 passed, zero failed, 37 intentional skips; coverage is 85.8% lines / 80.1% branches. These counts include changes already present in the live PR head before the review fixes. No inline review threads remain unresolved.

Usage Example

# Produce both architectures and all packages before tests.
.\scripts\build-cli.ps1 -SkipTests -SkipDocs

# Validate without republishing or deleting those packages.
.\scripts\build-cli.ps1 -OnlyTests -UseExistingArtifacts

CI selects -TestSuite Cli -CliShard 1, -TestSuite Cli -CliShard 2, -TestSuite Auxiliary, and -TestSuite UIAutomation on separate runners, never concurrently in one checkout.

Related Issue

N/A

Type of Change

• 🔧 Config/build
• ♻️ Refactoring
• 🧪 Test update
• 📝 Documentation

Final measured result

Run 35290431008, HEAD 256db41b: all build, validation, sample and aggregate checks passed.

Metric	Final result
All packages downloadable, from runner start	9m 21s
All packages downloadable, including startup/queue	9m 26s
Full merge-gating workflow	27m 54s
Total job execution	129.90 runner-minutes
CLI shard jobs	10m 02s / 13m 14s
Auxiliary / UIAutomation jobs	5m 22s / 5m 48s

The original observed workflows withheld downloads until roughly 30+ minutes. The successful intermediate Core/UI comparisons took 37m 47s / 121.0 runner-minutes and 39m 41s / 136.95 runner-minutes. The final run finished 9m 53s sooner than the closer comparison, while using 8.9 more runner-minutes. Across-run restore/runtime/sample variation means these are observed comparisons, not controlled benchmark guarantees. Electron was the final run's longest downstream job at 17m 52s.

The removed architecture experiment made packages available in 9m 30s versus 9m 52s–10m 02s in then-current serial-publish runs, but cost about five extra artifact runner-minutes. That variance-sized latency difference did not justify the extra orchestration.

Verification

Final hosted run:

• 7,215 .NET cases: 7,178 passed, zero failed, 37 intentional skips. Both CLI shards account for 6,564 distinct test IDs with zero overlap, plus 651 UIAutomation cases.
• Node 307 passed; analyzer 65 passed; stand-down contract passed; NuGet Pester 62 passed; scripts Pester 228 passed.
• All 14 samples passed and uploaded their JUnit reports. UI E2E/coordination, docs, metrics and both aggregate gates passed.
• Merged coverage: 85.7% lines / 79.9% branches. Exactly three TRX reports are included once; coverage is unioned across overlapping sources.
• CodeQL and the other completed PR checks passed; the review thread is resolved.

Local final validation also passed 228 script regressions, real Windows package builds, generated docs/schema checks and actionlint. A containment test now uses the existing architecture-aware published-binary helper rather than choosing the newest bin executable, avoiding ARM64 selection on x64. No product code changed.

Issues found and resolved during measurement

The initial shard minimum used the discovered count with MTP's --minimum-expected-tests, but that option counts executed tests and excludes intentional skips. It was corrected to minimum 1 with separate exact TRX accounting. Real passing-plus-skipped and all-skipped MTP runs verified the semantics; regressions still reject missing cases, partial reports, all-skipped runs, failed/nonterminal states and every nonzero process exit.

An unchanged native Media Foundation test failed once in the expanded experiment. It passed in both later hosted runs, including the final green run; its original cause remains unproven. No test was retried away, skipped or weakened to hide it. Failed intermediate runs remained red and retained diagnostic reports.

Checklist

• Early artifacts, exhaustive sharding and strict report/coverage invariants
• Measured experiments and user-approved removal of architecture splitting
• Final local build/docs/regression validation
• Final HEAD hosted run completed successfully with all suites and reports accounted for
• Required validation checks green; review feedback resolved

MainProtection's exact build-and-package context remains. The branch is behind newer main changes, which were deliberately not chased by repeatedly canceling measurements. A current-base update may be required by strict branch protection before landing. The PR remains open and unmerged.

[Copilot]

🟡 Changes recommended

Failed validation on main can replace the successful metrics baseline used for future PR comparisons.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Publishes build artifacts early, then reuses them across independent validation jobs.

Changes:

• Adds artifact-production and reuse modes with regression coverage.
• Refactors validation, samples, metrics, and aggregate checks.
• Documents early artifacts as awaiting validation.

File summaries

File	Description
scripts/build-cli.ps1	Adds artifact reuse and separated validation.
scripts/package-npm.ps1	Validates generated npm commands before packaging.
scripts/tests/build-cli.Tests.ps1	Tests build modes and failure handling.
scripts/tests/artifact-workflow.Tests.ps1	Tests workflow dependencies and metrics gates.
.github/workflows/build-package.yml	Splits artifact production from validation.
.github/workflows/test-samples.yml	Reuses same-run packages for sample tests.
.github/workflows/post-metrics-comment.yml	Clarifies validation and test status.
.github/actions/collect-metrics/action.yml	Requires complete test reports.
README.md	Documents early PR downloads.
AGENTS.md	Documents the new build and CI workflow.

Review details

• Files reviewed: 10/10 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

Build Metrics Report

Binary Sizes

Artifact	Baseline	Current	Delta
CLI (ARM64)	56.89 MB	56.89 MB	✅ 0.0 KB (0.00%)
CLI (x64)	56.94 MB	56.94 MB	✅ 0.0 KB (0.00%)
MSIX (ARM64)	23.65 MB	23.65 MB	📉 -0.0 KB (-0.00%)
MSIX (x64)	25.10 MB	25.10 MB	📈 +0.2 KB (+0.00%)
NPM Package	49.33 MB	49.33 MB	📈 +0.0 KB (+0.00%)
NuGet Package	49.42 MB	49.42 MB	📈 +0.3 KB (+0.00%)

Test Results

✅ 7494 passed, 37 skipped out of 7531 tests in 1680.4s (+180.7s vs. baseline)

Test Coverage

✅ 85.8% line coverage, 80.1% branch coverage · ⚠️ -0.2% vs. baseline

CLI Startup Time

49ms median (x64, winapp --version) · 📉 -13ms vs. baseline

Try This Build

Installs the MSIX for your architecture, replacing any previously installed build. Needs the GitHub CLI — the command offers to install it and sign you in if it is missing.

& ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) 870

Switching between builds often?

Put the tool on your PATH once:

& ([scriptblock]::Create((irm https://raw.githubusercontent.com/microsoft/winappCli/main/scripts/winapp-pr.ps1))) -AddToPath

Then this build is just:

winapp-pr 870

Run winapp-pr with no arguments to pick from a list of open PRs.

---

Updated 2026-09-18 20:00:31 UTC · commit bc0dece · workflow run

[Copilot]

🔵 Needs a closer look

The required-check orchestration and artifact trust boundaries warrant final human verification despite comprehensive tests and a green hosted run.

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Alexandre Zollinger Chohfi (azchohfi)]

Re-running a single failed lane fails on the artifact name, not the tests

What is wrong: Splitting validate-tests into four independent lanes makes it possible, for the first time, to re-run just the lane that flaked instead of rebuilding everything. That doesn't work yet. GitHub artifact names are immutable for the lifetime of a workflow run — not per run attempt — so actions/upload-artifact fails with 409 Conflict: an artifact with this name already exists when a re-run re-executes an upload step that already succeeded on the first attempt. None of the upload steps in build-package.yml set overwrite: true.

Artifacts have to persist across attempts, incidentally — that is exactly why the re-run lane can still download cli-binaries from a build-artifacts job that wasn't re-run. The persistence that makes granular retry possible is the same persistence that blocks the upload.

Show me:

1. validate-tests (Cli-2) hits a flaky test. scripts/test-cli-shard.ps1 writes its TRX before exiting non-zero, so artifacts/TestResults/ is populated, and the upload step is if: ${{ !cancelled() }} — so the artifact validation-results-Cli-2 is created even though the lane is red (build-package.yml:155-161).
2. Click Re-run failed jobs.
3. The lane re-runs and the tests now pass. ✅
4. The upload step fails: 409 Conflict: an artifact with this name already exists on the workflow run.
5. The lane is red again for an unrelated reason, so build-and-package stays red. Repeating the re-run repeats the conflict.

Expected: re-running the failed lane replaces its results artifact and the run can go green.

The same applies to the combined test-results upload in metrics (build-package.yml:266-273), which on a pull request runs even after a failed lane, and to the two if: always() uploads in e2e-test-ui (build-package.yml:320-334).

Note the shape of it: the conflict only occurs when tests genuinely ran and one failed — precisely the case worth retrying. If a lane dies before producing results, if-no-files-found: error fails the upload, no artifact is created, and the re-run is clean.

Why it matters: "Re-run all jobs" still works, because GitHub deletes the previous attempt's artifacts on a full re-run. But that re-runs build-artifacts and rebuilds everything, which is the cost this PR exists to remove. Without the fix, the only recovery from a flaky test is the full rebuild, so the headline benefit of the split is unreachable in the one situation that motivates it.

Smallest fix: add one input to the affected upload steps.

- name: Upload test results
  if: ${{ !cancelled() }}
  uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
  with:
    name: validation-results-${{ matrix.lane }}
    path: artifacts/TestResults/
    if-no-files-found: error
    overwrite: true

Same on test-results in metrics, and on ui-coordination-test-results / ui-test-screenshots in e2e-test-ui. overwrite: true deletes a matching artifact before uploading and does not fail when none exists, so first-attempt behavior is unchanged. build-artifacts' four package uploads don't need it — that job isn't re-run by a partial re-run, and a full re-run clears artifacts first — though adding it there too would be harmless.

Worth a regression test in scripts/tests/artifact-workflow.Tests.ps1, which already parses this workflow: assert that every upload-artifact step whose condition allows it to run after a failure also sets overwrite: true. That keeps a future upload step from reintroducing the dead end.

---

Related, lower severity: a failing test on main produces no test report

What is wrong: The combined test-results artifact is uploaded only by metrics, and metrics is skipped when validation fails on a non-pull-request run.

Show me: Push a commit to main that breaks a test → validate-tests fails → metrics is skipped, because build-package.yml:206 permits a failed validation only when github.event_name == 'pull_request' → the upload at build-package.yml:266-273 never runs → test-report.yml:13 still fires, since the run concluded failure rather than cancelled, and dorny/test-reporter errors with an artifact-not-found message. Expected: the report lists the failing tests.

Why it matters: On main this is exactly when the per-test report is needed, and instead there's a second red check with a misleading message. The previous single-job workflow uploaded test-results unconditionally (if: ${{ !cancelled() }}), so this is a change in behavior. Pull requests are unaffected.

Smallest fix: let metrics run on a failed validation for all events, and move the success-only condition onto the baseline cache write. Broadening the job alone would let a failed run overwrite the main metrics baseline.

[Nikola Metulev (nmetulev)]

Fixed both issues from your review in bc0dece. Every producer now overwrites only its own uniquely named artifacts, including samples and metrics-data. Failed main/manual validation publishes combined test-results, while successful-validation gating still protects main baseline promotion.

Verified run 35384710380: the full run passed; attempt 2 reran only Auxiliary and its dependent reporting jobs, replacing exactly Auxiliary/test-results/metrics-data while all 23 other artifact IDs and digests remained unchanged. Attempt 3 reran only packaging-cli and its aggregates, replacing only that sample report; all 25 siblings remained unchanged. Package producer timestamps were unchanged throughout—no rebuild and no 409 conflicts.

All 25 workflow regressions and actionlint passed, including failed-main reporting versus baseline-promotion conditions. The current-head CLI/docs build passed. Both issues are addressed; no validation gates or tests were removed.

[Alexandre Zollinger Chohfi (azchohfi)]

Verified bc0dece8. Both issues are closed, and the fix is tighter than what I proposed.

Artifact overwrite. All 12 uploads now set overwrite: true — the four package uploads, the lane results, combined test-results, both e2e-test-ui uploads, the three in test-samples.yml, and metrics-data in report-metrics/action.yml.

I checked the one way overwrite: true could make things quietly worse: test-samples.yml's build job also uploads npm-package and nuget-packages, so if it ran alongside build-artifacts the overwrite would replace a package artifact instead of failing loudly. It can't — that job is if: ${{ !inputs.use-existing-artifacts }} and build-package.yml:192 passes use-existing-artifacts: true, so it is skipped on the PR path. The new test asserts that guard rather than leaving it implicit.

Failed-main reporting. Moving the gate from the job to the Report build metrics step is the right seam. All three cache steps (Save metrics baseline, Delete old metrics baseline cache, Cache metrics baseline) live inside report-metrics and are already if: github.ref == 'refs/heads/main', so a red main publishes test-results while the baseline stays untouched.

I also confirmed test-results survives a collect-metrics failure: Upload combined test results precedes Collect build metrics, so a lane that dies without its TRX doesn't also cost the report. The test pins that ordering with an IndexOf comparison.

Verification on my side. Invoke-Pester -Path scripts\tests → 232 passed, 0 failed (up from 228; exactly the four new tests). The added coverage is stronger than the assertion I suggested: it pins the upload count at 12 and expands all 26 artifact names to check for duplicates, and it evaluates the real if: expressions from the YAML across every event × validation-result combination instead of string-matching them. Run 35384710380 reaching attempt=3, conclusion=success with 26 artifacts is the part that actually settles it — two partial reruns, no 409.

No further findings from me. 👍