ci(web): single-source toolchain versions, patch-level drift guard, Godot 4.7.1 - #8
Merged
Merged
Conversation
….7.1 bump Closes three gaps in the experimental C#/WASM export pipeline and adds a plan for getting it onto officially supported foundations. Fixes: - Production and preview could build different toolchains. The editor tag and template version were duplicated in four places (composite action defaults, workflow_dispatch defaults, and inline fallbacks in the production job) while the preview job passed nothing and inherited the action defaults. Updating one and not the others meant /preview validated a toolchain production never used. All versions now come from .github/web-toolchain.env; action inputs default to empty and act purely as deliberate overrides. - The drift guard compared only major.minor, so a 4.7.0-vs-4.7.1 mismatch passed silently -- the most likely kind of bump. It now normalises all three spellings of a version (4.7.1 / 4.7.1.stable.mono / 4.7.1-stable), requires exact agreement including the editor tag, and handles Godot's convention that x.y releases carry no patch component. It also verifies a checksum is pinned for the target asset before downloading a 165 MB editor rather than after. - Checksum verification matched the first zip found in the cache directory; it now requires the expected filename, so a restored cache cannot substitute a differently-named editor. Cache key includes the fork repo as well as the tag. Toolchain bump to 4.7.1-stable (upstream 4.7.1 is a stability-only release with no known incompatibilities with 4.7). Superseded checksums are retained so a rollback needs no checksum work. Docs: - docs/WEB_EXPORT_ROADMAP.md (new): upstream status with evidence, definition of done, four-phase migration plan, risk register, quarterly monitoring routine and rollback procedure. Records that LibGodot Core (#110863) merged and shipped in Godot 4.6, that the approach pivoted away from the stalled #106125 to LibGodot, and that #121502 and #118976 remain open at milestone 4.x -- so official C# web export is still unscheduled. - docs/WEB_EXPORT.md: corrected the DNS section (maze.ryankelly.dev resolves), softened the fork-abandonment risk (the fork matched upstream 4.7.1 within two days), and repointed the version matrix at the new single source of truth. - AGENTS.md: added the web-only failure modes (invariant globalization, missing crypto BCL APIs, no GDExtension), which pass desktop CI and break only in the browser. Not verified here: no .NET SDK in this environment, so the desktop build against Godot.NET.Sdk 4.7.1 is unbuilt. The version-resolution and drift-guard logic was executed against all drift permutations, and both editor checksums were computed from the published assets (the 4.7-stable hash reproduces the existing pin). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhKDz2xEVEtYECxT7q5F3
This was referenced Jul 25, 2026
…sm data race
`[assembly: Parallelizable(ParallelScope.All)]` was combined with NUnit's default
SingleInstance lifecycle, so test cases in a fixture ran concurrently against one
shared instance. Every field assigned in `[SetUp]` was therefore a data race.
It surfaced as two NullReferenceExceptions in RandomValueTests on CI, on a commit
whose other runs of the same job were green — re-running the identical commit
passed, confirming a race rather than an environment difference. The mechanism:
_mazePointFactory = new Mock<IMazePointFactory>(); // (a) bare mock
_mazePointFactory.Setup(x => x.MakePoint(...)).Returns(...); // (b) configures it
_randomPoint = new RandomPointGenerator(_random,
_mazePointFactory.Object); // (c) re-reads the field
Another test's (a) landing between this test's (b) and (c) makes (c) capture an
unconfigured mock. Moq then returns default(MazePoint) — null — and the test dies
dereferencing `point.X`, nowhere near the actual cause.
Measured with a trace harness (NUnit's per-test console capture reorders output and
hides this): with SingleInstance, four concurrent `[SetUp]` bodies share one fixture
instance; with InstancePerTestCase, four concurrent tests get four distinct
instances. Parallelism is unchanged, the unsafe sharing is gone.
Also drops MovementHelperTests' `[NonParallelizable]`, which was a local workaround
for this same root cause, and adds a guard test so removing the attribute fails
immediately by name instead of resurfacing as an occasional unexplained flake.
This branch and `main` had no Node or Playwright ignore rules, so nothing stopped an `npm ci` in tests/visual/ from being committed. That is exactly how 170 files / 17.7 MiB of node_modules got into the GitHub-Actions branch (#7), which is also based on `main`. Later branches in this stack do carry `tests/visual/`-anchored rules, added with the visual-regression harness. These are deliberately repo-wide rather than anchored: the trap is a node_modules appearing somewhere the anchored pattern doesn't cover. Baseline screenshots under *-snapshots/ stay tracked; only regenerated run output is ignored.
rtkelly13
force-pushed
the
claude/split-1-web-toolchain-3a6h5m
branch
from
July 26, 2026 08:21
419235e to
13e49ae
Compare
rtkelly13
pushed a commit
that referenced
this pull request
Jul 26, 2026
This branch committed 170 files / 17.7 MiB of tests/visual/node_modules, plus a Playwright test-results/.last-run.json, because neither this branch nor `main` had any Node or Playwright ignore rules. The blobs are removed from this branch's history rather than deleted in a follow-up commit, so they never reach `main` at all — a later deletion would leave 17.7 MiB permanently in the repository. Identical to the block added on the #8 stack, so whichever merges to main first, the other merges without conflict. Baseline screenshots under *-snapshots/ stay tracked.
rtkelly13
added a commit
that referenced
this pull request
Jul 26, 2026
* ci: upgrade GitHub Actions to current majors GitHub now warns that actions/checkout@v4 and actions/setup-dotnet@v4 target Node 20 and are being force-run on Node 24. This moves every action to its current major, all of which declare the node24 runtime. actions/checkout v4 -> v7 (5 uses) actions/setup-dotnet v4 -> v6 (2 uses) actions/setup-node v4 -> v7 (2 uses) actions/cache v4 -> v6 (1 use) actions/upload-artifact v4 -> v7 (1 use) These are multi-major jumps, so the release notes were checked for breaking changes rather than bumping blind. Four are relevant, none of them bite here: - checkout v5+ refuses to check out fork PR code by default, needing `allow-unsafe-pr-checkout: true` to restore the old behaviour. The guard fires only on `pull_request_target` and `workflow_run`. This repo triggers on push, pull_request, workflow_dispatch and issue_comment, so nothing changes -- notably the /preview job's PR-head checkout is unaffected. (That job's own trust boundary is unchanged by this upgrade: it still runs owner-only.) - setup-dotnet v5+ dropped support for older .NET versions. This repo pins 9.0.x and targets net8.0/net9.0, so it is unaffected. - setup-node v5+ enables automatic caching when package.json has a `packageManager` field, narrowed to npm in v6. No package.json in this repo sets that field, so no caching behaviour changes and no lockfile is required. - cache/upload-artifact v5+ require Actions Runner 2.327.1+. Only GitHub-hosted runners are used, which are well past that. Verified: fetched action.yml at each new major and confirmed every input this repo passes still exists (11 steps cross-checked programmatically), that cache@v6 still exposes the `cache-hit` output the export action reads, and that upload-artifact@v7 still accepts `if-no-files-found`. All three YAML files parse. Coverage note: only test.yml runs on a pull request, so these checks exercise checkout@v7 and setup-dotnet@v6 alone. The setup-node, cache and upload-artifact bumps live in web-export.yml, which runs only on merge to main or an explicit /preview -- they are validated by schema, not by execution. Worth a /preview before merging if you want them exercised first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhKDz2xEVEtYECxT7q5F3 * ci: pin actions to commit SHAs with version comments Adopts the pinning convention already used across the other repos in this account -- `uses: <action>@<40-char commit sha> # v<semver>` -- which this repo was the only one not following (0 of 13 pinned, against 100% in blog, github-actions, platform, rights-application, sentric-frontends, workspace and data-platform, and the large majority in the-vault-system and shared-utilities). A moving major tag can be repointed at new code by whoever controls the action, so the tag alone is not a supply-chain control. This repo already checksum-pins the third-party Godot editor binary for exactly that reason; the actions were the remaining gap. actions/checkout @3d3c42e5 # v7.0.1 (5 uses) actions/setup-dotnet @a98b5685 # v6.0.0 (2 uses) actions/setup-node @82076278 # v7.0.0 (2 uses) actions/cache @55cc8345 # v6.1.0 (1 use) actions/upload-artifact @043fb46d # v7.0.1 (1 use) Full semver in the comment, matching the dominant form in the other repos (platform, github-actions, the-vault-system) rather than the bare-major form in blog, since the exact patch is the thing a reader wants to check. Verification -- the failure mode of this convention is a comment that drifts from its SHA, so both directions were checked: - Resolved every SHA from upstream tags, then confirmed the method by resolving four pins that already exist in the other repos (checkout v7.0.0, setup-node v7.0.0, cache v6.1.0, setup-dotnet v5.2.0) and matching them byte for byte. - Re-derived each committed pin from its own comment: 5 distinct pins, 0 mismatched. - Two SHAs are byte-identical to pins already in platform (setup-node v7.0.0, cache v6.1.0), so those are corroborated by a second source. - All three YAML files parse; no unpinned third-party actions remain. Where this differs from the other repos: checkout and upload-artifact go to v7.0.1 rather than the v7.0.0 pinned elsewhere, and setup-dotnet to v6.0.0 rather than v5.2.0 -- these are the current latest, and a fresh pin may as well start there. Say the word to align them downward instead. No Dependabot config added: no repo in this account has one, so these pins are maintained by hand and this change does not alter that. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NwhKDz2xEVEtYECxT7q5F3 * chore: gitignore node_modules and Playwright run output This branch committed 170 files / 17.7 MiB of tests/visual/node_modules, plus a Playwright test-results/.last-run.json, because neither this branch nor `main` had any Node or Playwright ignore rules. The blobs are removed from this branch's history rather than deleted in a follow-up commit, so they never reach `main` at all — a later deletion would leave 17.7 MiB permanently in the repository. Identical to the block added on the #8 stack, so whichever merges to main first, the other merges without conflict. Baseline screenshots under *-snapshots/ stay tracked. --------- Co-authored-by: Claude <noreply@anthropic.com>
Each composite-action step now invokes a dedicated script under .github/actions/export-web/scripts/, keeping action.yml to wiring (env, ids, outputs). The drift-guard error message becomes a plain here-string now that it lives outside a YAML block scalar. All scripts parse clean; resolve-toolchain and check-version-drift verified locally including the drift-failure path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rtkelly13
added a commit
that referenced
this pull request
Jul 27, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rtkelly13
added a commit
that referenced
this pull request
Jul 27, 2026
AGENTS.md keeps this branch's testing/session sections and re-adds the Repository Conventions section from main (deduplicated). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
rtkelly13
added a commit
that referenced
this pull request
Jul 27, 2026
…a6h5m Both this branch and main had extracted the composite action's inline PowerShell into script files; this branch's extraction (17 scripts, mechanically lifted and diffed) supersedes main's 8, so its versions are kept and main's now-unreferenced download-editor.ps1 is removed. Re-adds main-only .gitignore entries and the AGENTS.md Repository Conventions section. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
rtkelly13
added a commit
that referenced
this pull request
Aug 1, 2026
…t rendering (#13) The 4.7.1 web export (from #8) ships with corrupted texture uploads: every texImage2D/bufferData call errors ('ArrayBufferView not big enough' / 'srcOffset + length too large'), leaving all UI text as garbled boxes on maze.ryankelly.dev. Bisected via preview-maze.ryankelly.dev: the 4.7.0 build of 038e196 renders cleanly with zero WebGL errors. Rolls GODOT_FORK_TAG, GODOT_TEMPLATE_VERSION and Godot.NET.Sdk back together per the drift guard. The 4.7-stable checksum entry was retained in editor-checksums.txt by design. Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack 1 of 4 — split out of #6, which had grown to cover four separate concerns. This is the original web-hosting work and stands alone on
main.Started as "has official Godot C# web export landed?" (it hasn't — see below) and turned into fixing three gaps the review surfaced in our own pipeline.
Gaps fixed
1. Production and preview could build different toolchains. The editor tag and template version were duplicated in four places — the composite action's defaults,
workflow_dispatchdefaults, and inline|| '4.7-stable'fallbacks in theproductionjob — whilepreviewpassed nothing and inherited the action defaults. Bumping one and not the others meant/previewvalidated a toolchain production never built. All versions now come from.github/web-toolchain.env.2. The drift guard couldn't catch a patch-level mismatch. It compared only
major.minor, so4.7.0vs4.7.1passed silently — the most likely kind of bump. It now normalises all three spellings (4.7.1/4.7.1.stable.mono/4.7.1-stable), requires exact agreement including the editor tag, and confirms a checksum is pinned before downloading a 165 MB editor.3. Checksum verification could be bypassed by the cache. It matched the first
*.zipin the cache directory rather than the expected filename. Now matched exactly, and the cache key includes the fork repo.Toolchain bump to 4.7.1-stable
Upstream 4.7.1 is a stability-only release (78 fixes, no known incompatibilities with 4.7). Superseded checksums are retained so a rollback needs no checksum work.
Upstream verdict
Official C#/.NET web export is still not supported, so the hack stays — but the strategy pivoted favourably: LibGodot Core (#110863) merged and shipped in Godot 4.6, with #121502 (LibGodot on web) and #118976 (.NET on top) still open at milestone
4.x. Direction is good; no date. Full status, exit criteria and migration plan in the newdocs/WEB_EXPORT_ROADMAP.md.Verification
testcheck green.Docs
docs/WEB_EXPORT_ROADMAP.md(new).docs/WEB_EXPORT.mdcorrected — DNS formaze.ryankelly.devresolves now, and the fork-abandonment risk is downgraded with evidence (it matched upstream 4.7.1 within two days).AGENTS.mdgains the web-only failure modes, which pass desktop CI and break only in the browser.Ordering
Merge #7 (GitHub Actions pinning) before or after this — both touch
test.yml/web-export.yml, so expect a small conflict either way. Then #6's stack: this → #10 → #11 → #6.Generated by Claude Code