Skip to content

ci: restore with NuGet lock files and cache the packages folder - #1293

Merged
github-actions[bot] merged 1 commit into
mainfrom
schedule/issue-sweep/2026-10-15/748-nuget-lock-files-ci-cache
Oct 15, 2026
Merged

github-actions[bot] merged 1 commit into
mainfrom
schedule/issue-sweep/2026-10-15/748-nuget-lock-files-ci-cache

Conversation

@JSdotNet

Copy link
Copy Markdown
Owner

Closes #748

Summary

Finishes the last open part of item 4. RestorePackagesWithLockFile is on in the root Directory.Build.props, and 85 packages.lock.json files are committed — every Backlog.sln project except the two MAUI heads, which opt out in their own csproj. The pull request dotnet job and nightly-tests.yml restore Backlog.WithoutAppHeads.slnf with --locked-mode (NU1004 on drift), and setup-dotnet in both workflows caches the NuGet folder keyed on **/packages.lock.json. The release and deploy workflows stay unlocked and uncached. BuildPolicyTests and ContinuousIntegrationWorkflowTests pin all of it, and guideline ADR 0002 records the heads exception and the alternative. Items 1–3 were already on main (#808, #936).

Decision taken without asking (follow it, or ask for the alternative)

Question. How should NuGet lock files and the setup-dotnet cache handle the MAUI heads, whose restore graph depends on the runtime identifier, and which CI jobs should enforce and cache them?

Chosen. Lock files for every Backlog.sln project except the two MAUI heads. The root Directory.Build.props turns the property on, and each head csproj turns it off explicitly. Restore in locked mode only in the two jobs that restore Backlog.WithoutAppHeads.slnf on windows-latest: the PR dotnet job and nightly-tests.yml. Set cache: true on setup-dotnet in pull-request.yml (both jobs) and nightly-tests.yml. Leave the release and deploy jobs uncached and unlocked.

Alternative. Per-RID handling. Give the heads lock files too and normalise their restore inputs: declare a fixed RuntimeIdentifiers list in each head (win-x64 for Desktop, the Android RIDs for Mobile). Replace the app-heads job's -r win-x64 second restore with a single RuntimeIdentifierOverride restore. Run every head restore, the release workflows included, in locked mode, so all 87 projects are locked.

Why the choice. The heads are where restore inputs vary today. The Desktop head is restored once with RuntimeIdentifierOverride=win-x64 and once with -r win-x64 (pull-request.yml app-heads job), and published with an override and MSIX properties in release-desktop.yml. The Android head pulls in the Android RuntimeIdentifiers and is restored on ubuntu in release-mobile.yml. Locking them means changing how the release pipelines restore, inside a change whose stated value is reproducible test restores and a faster PR run. The heads have no tests and sit outside the test suite's restore. The 85 non-head projects are what the PR and nightly suites restore, and they are what the cache speeds up. Locked mode belongs only where the restore inputs are fixed: Backlog.WithoutAppHeads.slnf on windows-latest x64. That runner matches the win-x64 host RID the AppHost's implicit Aspire.Dashboard.Sdk.* and Aspire.Hosting.Orchestration.* references record (Aspire.AppHost.Sdk 13.6.1 Sdk.targets lines 194-195). The app-heads job's global -r win-x64 would flow into the referenced projects and fail their lock files, so that job stays unlocked. Release and deploy jobs stay uncached so a stale or poisoned cache cannot reach a shipped artifact.

To follow the alternative instead. For the alternative, the PR would also need: a RestorePackagesWithLockFile opt-in and explicit RuntimeIdentifiers in both head csproj files instead of the opt-out; two committed head lock files; app-heads restores reduced to one RID-consistent restore per head with --locked-mode; locked-mode restores, and probably caches, added to release-desktop.yml and release-mobile.yml; the ArchitectureTests changed from 'heads opt out' to 'every project has a lock file'; and ADR 0002's deviation entry for the heads removed.

Acceptance criteria

    1. Directory.Build.props sets RestorePackagesWithLockFile=true unconditionally with a comment naming the heads exception and the AppHost constraint — test: BuildPolicyTests.The_root_props_turns_lock_files_on_for_every_project (the value and the missing condition; the comment's content was checked by reading only)
    1. Desktop and Mobile csproj set RestorePackagesWithLockFile=false with a rationale comment and have no packages.lock.json — test: BuildPolicyTests.Each_app_head_opts_out_of_lock_files (both heads; the comment checked by reading only)
    1. Every other Backlog.sln csproj (85) has a committed packages.lock.json beside it — test: BuildPolicyTests.Every_solution_project_but_the_app_heads_commits_its_lock_file (checks the file exists on disk; committing is for the owning session)
    1. dotnet restore Backlog.WithoutAppHeads.slnf --locked-mode succeeds with no NU1004, and a second plain restore leaves the lock files unchanged — not proved by a test: Verified by hand: locked-mode restore exit 0; the combined SHA1 of the 85 lock files stayed the same after a plain restore, a --force-evaluate restore and a full Backlog.sln restore
    1. pull-request.yml dotnet job and nightly-tests.yml restore the slnf with --locked-mode — test: ContinuousIntegrationWorkflowTests.The_solution_filter_restores_in_locked_mode (pull-request.yml/dotnet, nightly-tests.yml/dotnet)
    1. pull-request.yml app-heads job restores without --locked-mode — test: ContinuousIntegrationWorkflowTests.The_app_heads_restore_without_locked_mode
    1. Every setup-dotnet in pull-request.yml (both jobs) and nightly-tests.yml sets cache: true keyed on **/packages.lock.json; release-desktop, release-mobile, deploy-sync setup-dotnet unchanged — test: ContinuousIntegrationWorkflowTests.The_test_runs_cache_nuget_packages_and_the_release_runs_do_not
    1. BuildPolicyTests facts fail when the root property is missing, a non-head project has no lock file, or a head lacks its opt-out — test: BuildPolicyTests.The_root_props_turns_lock_files_on_for_every_project / Every_solution_project_but_the_app_heads_commits_its_lock_file / Each_app_head_opts_out_of_lock_files: all failed before the change
    1. ContinuousIntegrationWorkflowTests facts fail when the PR dotnet or nightly restore drops --locked-mode, or a setup-dotnet step drops cache: true — test: ContinuousIntegrationWorkflowTests.The_solution_filter_restores_in_locked_mode and The_test_runs_cache_nuget_packages_and_the_release_runs_do_not: both failed before the change
    1. The existing timeout, nightly parallelism, coverage and Node pin facts still pass; items 1-3 not changed — test: ContinuousIntegrationWorkflowTests (all 12 cases pass, including Every_job_on_a_runner_has_its_timeout, A_nightly_run_tests_at_default_parallelism, The_pull_request_run_collects_and_summarises_coverage, Every_node_step_runs_one_node_version)
    1. dotnet build Backlog.sln and dotnet test Backlog.sln pass; CI-equivalent Release TreatWarningsAsErrors build of the slnf has zero warnings — not proved by a test: Verified by running. Build Backlog.sln: 0 warnings, 0 errors. Non-incremental Release -p:TreatWarningsAsErrors=true slnf build: 0 warnings. CI-equivalent serial test run: 16269 total, 0 failed, 16256 passed, 13 skipped. Default-parallel dotnet test --solution Backlog.sln reported 16269 total but 5, then 2, Desktop.UI timing tests failed under load; that module passes alone (5134/5134).
    1. ADR 0002 records the lock files and the locked CI restore under 'How Backlog applies it', and the heads exception and AppHost constraint under 'Deviations and gaps'; meta block valid — not proved by a test: Checked by reading, plus the devbook checks
    1. node .devbook/_tools/devbook-meta/build.mjs --check and node tools/devbook/check-metadata.mjs both pass — not proved by a test: Both run in the worktree: exit 0, 0 blocking (advisory warnings were already there)
    1. Commit and PR say Fixes #748 — not proved by a test: Not done here: this worker must not commit or open a PR; left to the owning session

Testing

  • Command: cd "D:/iswp/748-nuget-lock-files-ci-cache" && dotnet build Backlog.sln -nologo -v:minimal && dotnet test Backlog.sln --no-build
  • Passed 16253, failed 3; repair attempts 2.
  • Owner session, after the review fixes: dotnet test tests/Backlog.Desktop.UI.UnitTests re-run — 5134 passed, 0 failed (the three failures in the workflow's run were bUnit timing flakes in Devbook pane and search tests this change does not touch). Both devbook checks pass.

Review

  • Fixed in the owner session: minor — the new 'Deviations and gaps' bullet ended with a blank line that turned the section into a loose list; removed.
  • Left open, minor (.github/workflows/pull-request.yml:281): The app-heads setup-dotnet step uses cache: true with the same **/packages.lock.json key as the dotnet job, but the packages it restores are not in those lock files. Both jobs run on windows-latest, so they compute the same cache key and race to save it. Which one wins decides what the cache holds.
  • Left open, minor (tests/Backlog.ArchitectureTests/ContinuousIntegrationWorkflowTests.cs:338): The new SetupDotnetSteps(string) repeats what BuildPolicyTests already has as SetupDotnetSteps(string[]) built on Steps()/StepItem(), and the copy is narrower. Its regex only treats a line starting - name: at six-space indent as a step start. The existing helper also accepts - uses: items at any indent.
  • Left open, minor (tests/Backlog.ArchitectureTests/ContinuousIntegrationWorkflowTests.cs:320): Assert.Equal(2, SetupDotnetSteps(Workflow(PullRequestWorkflow)).Count) fixes how many setup-dotnet steps pull-request.yml has, which is a detail of the current file and not the rule being tested. Suppose a later PR adds a third .NET job to pull-request.yml and gives its setup-dotnet step cache: true and the lock-file path correctly. The cache rule is still met, but this fact fails.
  • Left open, minor (tests/Backlog.ArchitectureTests/ContinuousIntegrationWorkflowTests.cs:6): The class summary lists what the workflows promise (timeouts, nightly parallelism, one Node version, coverage) and says nothing else can prove these. The change adds locked-mode restore and lock-file-keyed NuGet caching to this class but leaves the summary unchanged, so it no longer covers the class's contents. A reader who checks the summary to find where the locked-mode rule is enforced will not find it here.

What could not be proved

  • The workflow reported red for three Backlog.Desktop.UI.UnitTests bUnit timing/render-order failures (DevbookSearchViewMeaningTests.Only_the_latest_question_is_drawn, two HomeDevbookPaneTests). The whole project passed on a re-run (5134/5134), and every test the change touches (BuildPolicyTests, ContinuousIntegrationWorkflowTests) passed in the original run.
  • Whether --locked-mode holds on the windows-latest runner can only be proved by this pull request's own CI. The lock files were written on one Windows x64 machine; the AppHost's lock file stays free of host runtime identifiers only while AspireUseCliBundle is on. Watch the dotnet job's restore for NU1004.
  • From now on a package change must commit the regenerated lock files with it — Dependabot does; check that the package-update schedule's pull requests do too, or their locked-mode restore fails.
  • The cache is cold until the nightly run on main saves it; the first cache hit shows on a pull request after that.

Merge Danger

Door. Two-way: a revert removes the lock files and the locked-mode flag. Nothing persisted outside the repository changes; a GitHub Actions cache entry simply goes unused.

Blast radius. repository — every pull request's dotnet job and the nightly run; a stale lock file now fails CI instead of building.

Assumptions taken instead of asking

  • I assumed the projects that reference the AppHost (for example Backlog.EndToEndTests and others using Aspire.Hosting.Testing) carry the win-x64 Aspire packages into their own lock files, or at least that the AppHost's lock file does. A contributor restoring on arm64, macOS or Linux would then see a local diff in the lock files, but not a CI failure. I took that as acceptable because development and the locked CI jobs are Windows x64. Check the generated lock files: if this proves noisy, the AppHost could opt out like the heads.
  • I assumed setup-dotnet v6.0.0 accepts cache: true with the default cache-dependency-path (**/packages.lock.json) and finds the lock files from the repository root. If the default differs, set cache-dependency-path: '**/packages.lock.json' explicitly. Check this against the action's README at the pinned SHA.
  • I assumed SDK patch roll-forward (global.json latestPatch, 10.0.401) does not change the implicit package references recorded in the lock files. If a runner image's newer SDK patch changes them, the locked-mode restore fails with NU1004. The fix is to regenerate the lock files, which is the intended signal.
  • I assumed recording the decision in guideline ADR 0002's 'How Backlog applies it' and 'Deviations and gaps' sections is enough, rather than writing a new local ADR. Lock files apply central package management; they do not add a new architectural boundary. If the owner wants a standalone record instead, write a new local ADR at status proposed under .devbook/arc42/adr/ (next number 0029).
  • The scope's 'AppHost host-RID constraint' does not show up in practice. Backlog.Aspire.AppHost.csproj sets AspireUseCliBundle=true, and Aspire.AppHost.Sdk 13.6.1 Sdk.targets line 76 skips the implicit Aspire.Dashboard.Sdk. and Aspire.Hosting.Orchestration. references when it is set. The generated AppHost lock file therefore names no runtime identifier, and no lock file mentions win-x64. Directory.Build.props and ADR 0002 record the accurate constraint: the AppHost lock file is host-neutral only while AspireUseCliBundle is on, and turning it off would tie the file to the restoring machine's runtime identifier (win-x64).
  • Confirmed by experiment: the app-heads job's dotnet restore Backlog.Desktop.csproj -r win-x64 rewrites the lock files of the projects the head references, adding a net10.0/win-x64 section. That is why the app-heads job stays unlocked. On the runner the rewrite is harmless. A developer who runs that -r win-x64 restore locally gets modified lock files, which a plain dotnet restore Backlog.WithoutAppHeads.slnf puts back exactly (verified by hash). The AppHost's AddProject for the desktop head passes no runtime identifier, so aspire start does not cause this.
  • I set cache-dependency-path to '**/packages.lock.json' explicitly, although that is setup-dotnet's default, so the cache key is visible in the workflow and the test can pin it.
  • The dotnet and app-heads jobs both run on windows-latest and compute the same cache key. Whichever job saves first wins, so the other job may restore from a partial cache; NuGet just downloads what is missing. The heads' own packages (MAUI, Android) are not in any lock file, so a bump to a head-only package does not change the key. Both effects only cost speed.
  • I gave the release and deploy workflows a test asserting they stay uncached, which pins the decision to leave them uncached and unlocked. Rationale: a signed artifact is always built from packages fetched for it.
  • Lock files are left with CRLF, as NuGet writes them on Windows. With core.autocrlf=true, git stores them as LF. NuGet compares lock files by content, not bytes, so a locked restore passes and no rewrite happens with either line ending (both tested).
  • The full-parallel dotnet test --solution Backlog.sln failures were Backlog.Desktop.UI.UnitTests timing tests, most marked 'canceled'; a different set failed on each of two runs. I treated them as known load-dependent flakes unrelated to this change: the module passes alone (5134/5134), and the CI-equivalent serial run passes 16256 tests with 0 failures. I did not run a baseline on main to confirm they also flake there.
  • AC 14 (Fixes #748 in the commit and PR) is left to the owning session, because this worker was told not to commit or create a PR.

Opened as a draft by an attended run of delivery-schedule:schedule-issue-sweep (2026-10-15), told by the owner to decide without asking and to name the choice and the alternative.

🤖 Generated with Claude Code

Turns RestorePackagesWithLockFile on at the root and commits a lock
file for every project except the two MAUI heads, whose restore graph
changes with the runtime identifier. The pull request dotnet job and
the nightly run restore in locked mode, and setup-dotnet caches the
NuGet folder keyed on the lock files. Guideline ADR 0002 records the
heads exception; architecture tests pin the rules.

Decision taken without asking: lock everything but the heads rather
than normalising the heads' runtime identifiers; the alternative is
named in the pull request and in ADR 0002.

Refs #748

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@JSdotNet
JSdotNet marked this pull request as ready for review October 15, 2026 18:08
@github-actions
github-actions Bot merged commit 03bb97e into main Oct 15, 2026
15 checks passed
JSdotNet added a commit that referenced this pull request Oct 15, 2026
…2026-10-15/753-credentials-at-rest

Keeps both sides of the ADR list and related: conflicts (ADR 0030 beside main's ADR 0029/0031 and the Sync context's moved pairing anchors), and regenerates the packages.lock.json files main's #1293 committed, which the ProtectedData references this branch adds made stale.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add job timeouts, coverage collection and a parallel test run to CI

1 participant