Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions docs/foundation-checklist.md
Original file line number Diff line number Diff line change
Expand Up @@ -406,21 +406,25 @@ This step does not block engine development or justify further deployment expans
- [x] Replace accidental SDK drift from `global.json`'s `latestMinor` roll-forward
with a deliberate supported baseline for formatting, analyzers, and builds.
Log the selected SDK and update it through reviewed dependency changes.
- [x] Introduce committed NuGet lockfiles for application entry points and locked
- [x] Introduce committed NuGet lockfiles for shipped application entry points and locked
CI restores after SDK selection is settled. Keep npm tools exactly pinned with
committed lockfiles. Document how intentional dependency updates refresh them;
a library lockfile does not constrain downstream consumers.

**Acceptance:** a clean checkout runs the documented commands with the declared
toolchain. Missing prerequisites produce useful diagnostics, CI rejects dependency
drift, and local/CI checks have equivalent scope. A fresh editor rehearsal remains
toolchain. Missing prerequisites produce useful diagnostics, CI rejects missing
or stale shipped-application locks, and local/CI checks have equivalent scope. A fresh editor rehearsal remains
the separate acceptance in step 5. See [NuGet lockfiles](https://learn.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#locking-dependencies).

**Result (2026-09-22):** The read-only doctor and shared formatting, quick, full,
and browser commands are documented in [development setup](setup/development.md)
and used by CI. SDK selection is exact; application and test entry points have
committed locks, including separate Windows/Linux AppHost graphs. Locked restores
also cover CodeQL and packaging. A fresh source snapshot passed the full local
and used by CI. SDK selection is exact; the shipped API and Web Client have
committed locks covering CI, CodeQL, and packaging. Tests and the local-only
AppHost restore normally and remain in the solution build/test gates; their
transitive dependency graphs are not frozen. This avoids maintaining custom
platform locks or a bot that repairs dependency PRs; see the
[dependency-update rationale](quality/dependency-update-research.md).
A fresh source snapshot passed the full local
gate with serialized MSBuild: zero build warnings, 72 tests, frontend packaging,
smoke-package checks, and all five shell suites. Missing prerequisites, incorrect
versions, and missing/stale dependency locks were rejected. The
Expand Down
124 changes: 124 additions & 0 deletions docs/quality/dependency-update-research.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
# Dependency-update lockfile research

## Question and observed failure

Researched on 2026-09-22.

[PR #115](https://github.com/OpenGameBuilder/opengamebuilder/pull/115) updated
centrally managed OpenTelemetry versions, refreshed the API lock,
and left the API integration-test lock and the two AppHost RID-specific locks
stale. [Locked restore then failed](https://github.com/OpenGameBuilder/opengamebuilder/actions/runs/35745905146/job/106807351851).
A fresh regeneration of those three locks in a temporary copy of the CI merge
commit `a74bd33288f1c6d0c57ca72a99a3c0625f7c4fd4`
followed by locked restore succeeds for both the Windows and Linux-selected
AppHost graphs. The repository now uses a narrower policy that keeps committed
locks for the shipped API and Web Client while allowing tests and the local-only
AppHost to resolve dependencies normally.

## Evidence

NuGet documents that a lock captures the resolved dependency graph, including
changes from a dependent project's files, and that locked restore rejects an
inconsistent graph. Its default lock name is `packages.lock.json`; a project can
set `NuGetLockFilePath` or use `--lock-file-path` to select a different path.
[NuGet lock files](https://learn.microsoft.com/nuget/consume-packages/package-references-in-project-files#lock-file-extensibility)

Dependabot issue [#13950](https://github.com/dependabot/dependabot-core/issues/13950)
is open and describes this exact Central Package Management plus
`ProjectReference` case: it updates some locks but not a downstream project's
lock, which makes `RestoreLockedMode=true` fail with NU1004. As of this research,
the issue has no resolution or linked fix.

GitHub supports the `nuget` Dependabot ecosystem, but its public configuration
reference has no option to run a post-update restore, select a NuGet lock-file
path, or enumerate custom/RID lock files.
[Dependabot options reference](https://docs.github.com/en/code-security/reference/supply-chain-security/dependabot-options-reference)
This is evidence that there is no documented `dependabot.yml` remedy, rather
than proof that no internal behavior could change.

The current Dependabot NuGet updater itself runs a forced restore for a project
when it refreshes a lock.
[LockFileUpdater.cs at 098c329](https://github.com/dependabot/dependabot-core/blob/098c32944988dba7a697eda66da30c2a62b1dff6/nuget/helpers/lib/NuGetUpdater/NuGetUpdater.Core/Updater/LockFileUpdater.cs)
That does not resolve #13950's incomplete downstream selection or establish
support for this repository's custom AppHost paths.

Renovate documents NuGet `packages.lock.json` maintenance, central package
versions, and its source builds a `ProjectReference` dependency tree before
restoring dependent projects.
[Renovate NuGet manager](https://docs.renovatebot.com/modules/manager/nuget/)
[artifact updater at 3b70d5f](https://github.com/renovatebot/renovate/blob/3b70d5ffe2c2a0d5284f57d40fa687c751b56f0b/lib/modules/manager/nuget/artifacts.ts)
However, the same source derives only sibling `packages.lock.json` paths.
Therefore current source supports the test's ordinary lock more directly than
Dependabot, but does not establish support for `NuGetLockFilePath` or the
AppHost's `packages.win-x64.lock.json` and `packages.linux-x64.lock.json`.
This is a source-based inference, not a Renovate compatibility guarantee.

## Implemented policy

The lock policy is now limited to the shipped API and Web Client. Central
package versions, exact SDK selection, their committed lockfiles, and locked
packaging remain in place. Tests and the local-only AppHost no longer opt into
lockfiles, and their former locks are not retained. The AppHost remains in the
solution build, and both test projects still run in CI.

When the solution is restored with `--locked-mode`, NuGet rejects a stale or
missing lock for the opted-in API and Web Client. Projects that do not opt into
lockfiles use normal dependency resolution during that same restore. The
repository does not suppress NU1004 or keep ignored development locks.

The tradeoff is reduced repeatability for test and AppHost transitive dependency
resolution. Their resolved graphs can differ between restores and from the
shipped application graphs. Their builds and tests still run; production
dependency drift still fails locked restore. For this repository's current size,
that is a proportionate tradeoff and does not require a privileged
dependency-repair service. This policy is a project-specific judgment, not a
NuGet rule that test executables should never have lockfiles.

This removes the lockfile failure patterns demonstrated by PR #115. It does not
guarantee that Dependabot will correctly update every future production lockfile
or that a package or SDK update will pass all checks. A failure in either area
still requires investigation.

## Automation boundary

No bot write-back automation is configured. Reconsider complete lock
regeneration only if reproducible test or orchestration graphs become a
demonstrated requirement, or if ordinary shipped-application locks repeatedly
need completion and the manual cost warrants an owned automation. Such a system
would add credential maintenance, changed-head handling, and CI-trigger
behavior.

GitHub documents that PR events created by a workflow using `GITHUB_TOKEN`
require approval to run the resulting workflows; a GitHub App installation
token can trigger those checks automatically.
[Workflow triggering rules](https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/trigger-a-workflow#triggering-a-workflow-from-a-workflow)
Therefore fully unattended write-back would need an authentication or explicit
dispatch design beyond a lock-refresh script. CI validates the committed result
rather than silently repairing locks inside its workspace.

## Validation evidence

A fresh temporary copy of the exact PR #115 CI merge commit applied the narrowed
policy while leaving both shipped-application locks unchanged. On Windows,
`pwsh ./scripts/check.ps1 quick -Serial` passed its restore, C# formatting,
zero-warning Release build, and all 72 tests. Solution restore with
`--locked-mode` also passed with the default Windows SDK RID and with an
explicit `NETCoreSdkRuntimeIdentifier=linux-x64`; no development lockfiles were
regenerated. A deliberately mismatched API dependency still failed with NU1004,
confirming that the shipped-application guard remained active.

Missing API and Web Client locks were each rejected without regenerating them,
and a stale API lock was rejected. This historical snapshot establishes the
policy's behavior for the failure that motivated the change. It is not a
Linux-host execution or a hosted Dependabot rehearsal.

The implemented policy subsequently passed the repository's full local check,
including all 72 tests, frontend packaging, content and CI policy regressions,
and all five shell suites. The API and Web Client lockfiles were unchanged.

## Limits

This research does not claim that a GitHub App, Renovate deployment, or future
upstream release is configured or suitable here. Re-evaluate the decision when
Dependabot #13950 is resolved or when a dependency manager documents support
for custom NuGet lock paths and multi-RID regeneration.
41 changes: 32 additions & 9 deletions docs/quality/testing.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,11 @@ it cannot silently skip validation. The selector logs the paths and decision.
| Documentation | Ubuntu runs `check.ps1 content`: first-party formatting, lint, workflow/shell checks, and all local Markdown links/anchors. Checking all documents catches backlinks broken by deletions or renames. No solution build or container runs. |
| Full | Windows runs `check.ps1 quick -Serial` for locked restore, C# format, Release build, and tests. Ubuntu runs `check.ps1 full`, then builds and checks the API container. |

Both platforms build the AppHost with their committed platform-specific locks.
Both platforms restore and build the entire solution, including the AppHost,
and run both test projects. Only the shipped API and Web Client have committed
NuGet locks. Tests and the local-only AppHost resolve their dependencies normally;
their transitive graphs are not frozen. See the
[lockfile policy](../setup/development.md#command-line-workflow-start-here).
The shared action installs the exact SDK in a fresh runner-temporary directory
using [`DOTNET_INSTALL_DIR`](https://github.com/actions/setup-dotnet#environment-variables).
This prevents preinstalled Visual Studio workload manifests from selecting an
Expand Down Expand Up @@ -269,18 +273,37 @@ locked restore, format verification, a Release build with zero warnings, all 72
dependencies and syntax, and all five isolated shell suites. The serialized
option avoided a Windows MSBuild task-host failure; it did not omit checks.

Both Windows and Linux AppHost graphs passed locked restore; the Linux graph was
selected explicitly on Windows, not executed on a Linux host. Deliberately
missing entry-point locks and changed package requirements stopped the shared
quick command at restore, before build. Doctor fixtures rejected a missing SDK,
Deliberately missing shipped-application locks and changed package requirements
stop restore before build. Doctor fixtures rejected a missing SDK,
wrong SDK selection/policy, Node 24, and non-exact or mismatched Playwright pins.
Missing smoke URL inputs failed before launching Chromium. PowerShell/YAML
parsing, changed documentation targets/anchors, and diff checks passed.

The shared commands and workflow wiring have local evidence. Hosted `build-test`,
CodeQL, Docker image packaging, and live browser smoke after these changes remain
unverified. No services, deployments, certificate trust changes, or editor
rehearsals were performed for this validation.
These were local command checks. Later hosted platform evidence is recorded in
[platform CI acceptance](#recorded-platform-ci-acceptance). No services,
deployments, certificate trust changes, or editor rehearsals were performed for
this local validation.

### Shipped-application lock policy validation

The narrower policy was tested against a fresh copy of the failing
[PR #115 CI revision](https://github.com/OpenGameBuilder/opengamebuilder/actions/runs/35745905146/job/106807351851)
with its OpenTelemetry 1.19.1 update. Removing the test and AppHost lock opt-ins
and their four locks made solution restore pass with both Windows and
Linux-selected SDK RIDs. These restores ran on Windows; they are not Linux-host
execution evidence. The same snapshot passed `check.ps1 quick -Serial`, including
format verification, a Release build with zero warnings, and all 72 tests.

The implementation also passed `check.ps1 full -Serial` locally: the solution
gate, content checks and regressions, frontend publish/portability checks,
smoke-package checks, and all five isolated shell suites. Both committed
production lockfiles remained unchanged.

Missing API and Web Client locks were each rejected before restore could
regenerate them. A deliberately stale API dependency was rejected with NU1004.
No test or AppHost lockfiles were regenerated. These checks verify that the
shipped applications retain their lock guards while development graphs restore
normally; they do not guarantee that every future Dependabot update succeeds.

### Browser-smoke dependency updates

Expand Down
52 changes: 24 additions & 28 deletions docs/setup/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,9 @@ access are not required.** Aspire is the local launcher, not the production
deployment mechanism.

When updating the SDK requirement in `global.json`, review the compatible pinned
SDK image in the API Dockerfile and the generated dependency lockfiles together.
Dependabot SDK updates are reviewed with those files; CI builds the API image
without pushing it.
SDK image in the API Dockerfile and the two shipped-application dependency
lockfiles together. Dependabot SDK updates are reviewed with those files; CI
builds the API image without pushing it.

## Command-line workflow (start here)

Expand All @@ -65,10 +65,13 @@ Use `-Scope quick`, `full`, `content`, `format`, `browser`, or `development` whe
workflow.

The normal solution gate is `pwsh ./scripts/check.ps1 quick`: it runs the quick
doctor check, a locked restore, C# formatting verification, a Release build, and
the current 72 solution tests. `check.ps1 format` verifies C# after locked restore
and checks first-party content with Prettier and shfmt. To apply formatter changes
deliberately, run `pwsh ./scripts/check.ps1 format -Fix`, then review the diff.
doctor check, a solution restore with `--locked-mode`, C# formatting
verification, a Release build, and the current 72 solution tests. That restore
enforces the committed API and Web Client graphs; tests and the local-only
AppHost do not opt into lockfiles and resolve dependencies normally.
`check.ps1 format` verifies C# after the same restore and checks first-party
content with Prettier and shfmt. To apply formatter changes deliberately, run
`pwsh ./scripts/check.ps1 format -Fix`, then review the diff.
Install the [content-checking prerequisites](../quality/content-checks.md) before
using `format`, `content`, or `full`:

Expand Down Expand Up @@ -102,36 +105,29 @@ test project, replace `--solution opengamebuilder.slnx` with, for example,

Direct `dotnet restore`, `build`, `test`, and `format` commands remain useful for
focused editor work. Use `--locked-mode` for a manual restore that must reproduce
the committed graph. After an intentional SDK or
the shipped applications' committed graphs. After an intentional SDK or
[`Directory.Packages.props`](../../Directory.Packages.props) change, refresh
locks with:
those locks with:

```pwsh
dotnet restore opengamebuilder.slnx --force-evaluate -p:RestoreLockedMode=false
```

Review every generated lockfile and run the full check before committing. See
Review any lockfile changes and run the full check before committing. See
[NuGet's lock-file documentation](https://learn.microsoft.com/nuget/consume-packages/package-references-in-project-files#locking-dependencies)
for the restore model. The API, Web Client, and two test entry points commit
for the restore model. The shipped API and Web Client entry points each commit
`packages.lock.json`; shared-library locks cannot constrain the graph selected
by a downstream consuming application, so shared libraries do not duplicate
them. The AppHost's implicit SDK packages vary by host, so it commits reviewed
`packages.win-x64.lock.json` and `packages.linux-x64.lock.json` baselines. For an
intentional dependency refresh, update the native graph with the solution command
above, then refresh the Linux AppHost graph and confirm the native graph remains
locked:

```pwsh
dotnet restore src/OpenGameBuilder.AppHost/OpenGameBuilder.AppHost.csproj --force-evaluate -p:RestoreLockedMode=false -p:NETCoreSdkRuntimeIdentifier=linux-x64 -m:1
dotnet restore opengamebuilder.slnx --locked-mode
```

Refresh each AppHost lock on its matching host, or review an explicit
cross-target restore as above. A new host platform needs its own reviewed
AppHost lock before it is supported. `Directory.Build.targets` rejects a missing
entry-point lock before a locked restore can create one; `--locked-mode` then
rejects stale dependency graphs. CI restores with `--locked-mode`, and packaging's
implicit restore is locked as well.
them. Tests and the local-only AppHost use normal dependency resolution, so
their transitive graphs can change between restores. This gives up fixed
development-only graphs while retaining Central Package Management, exact SDK
selection, and locked shipped-application restore and packaging.

[`Directory.Build.targets`](../../Directory.Build.targets) rejects a missing
lock for an opted-in project before a locked restore can create one;
`--locked-mode` then rejects stale API or Web Client graphs. CI restores the
whole solution with `--locked-mode`, and packaging's implicit restore is locked
as well. No host-specific AppHost lock or lock refresh is required.

The root `NuGet.Config` deliberately has one source, `nuget.org`, and clears
both inherited package sources and inherited package-source mappings. Its `*`
Expand Down
3 changes: 0 additions & 3 deletions src/OpenGameBuilder.AppHost/OpenGameBuilder.AppHost.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,6 @@
<Sdk Name="Aspire.AppHost.Sdk" Version="13.4.2" />

<PropertyGroup>
<RestorePackagesWithLockFile>true</RestorePackagesWithLockFile>
<!-- Aspire restores host-specific dashboard and orchestration packages. -->
<NuGetLockFilePath>packages.$(NETCoreSdkRuntimeIdentifier).lock.json</NuGetLockFilePath>
<OutputType>Exe</OutputType>
<IsAspireHost>true</IsAspireHost>
<!-- Keep NuGet-restored orchestration dependencies for IDE and CI builds.
Expand Down
Loading
Loading