ci: pin the emscripten toolchain; stop the format gate swallowing rustfmt - #161
Merged
Conversation
…tfmt The wasm build installed `latest` and cached it under a constant key, so an expired cache silently swapped compilers. Emscripten's wasm exception-handling ABI has since moved, and the fresh `latest` fails to link with undefined __cpp_exception / _Unwind_* symbols — main is exposed to this on its next deploy, not just one branch. .emsdk-version is now the single source of truth: the build script installs that version and the deploy workflow keys its emsdk cache on the file's hash, so a cache miss reinstalls the same compiler and a bump invalidates the cache. A mismatched emscripten already on PATH warns (naming both versions and the link errors it causes) rather than failing, so developer machines still build. The format gate returned only prettier's status, so `cargo fmt --check` failures reported PASS — two files have been unformatted on main unnoticed and are reformatted here. Both checks now always run and either failing fails the gate. The gate's bare `cd` also leaked the working directory into every later gate; it and the site gate's now run in subshells.
|
🔍 Preview deployed: https://7c775899.toolpath.pages.dev |
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
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.
Two CI reliability bugs, both live on
mainright now.1. The wasm toolchain was unpinned
scripts/build-wasm.shbootstrapped withemsdk install latestanddeploy-site.ymlcached it under the constant keyemsdk-latest. That combination means the compiler is whateverlatestresolved to on the day the cache was last populated — and a cache eviction silently changes it.That eviction has now happened. Emscripten's wasm exception-handling ABI moved, and the fresh
latestfails to link:This is not branch-specific —
mainhits it on its next deploy.Fix:
.emsdk-versionis the single source of truth. The build script installs exactly that version; the workflow keys its emsdk cache on the file's hash, so a cache miss reinstalls the same compiler and bumping the pin invalidates the cache automatically. An emscripten already onPATHthat differs from the pin warns — naming both versions and the link errors the skew produces — rather than failing, so developer machines with their own SDK still build.Pinned to 5.0.1, verified rather than assumed: built this workspace on 5.0.1 with both the
--devprofile and the defaultwasmprofile CI actually runs (LTO,opt-level=z, strip). Both link clean, no__cpp_exception/_Unwind_*errors. For the record, a fresh emsdk clone currently mapslatest→ 5.0.1 while the failing CI run reportedreleases-833aa203…, which is precisely the skew being pinned out.2. The format gate reported PASS on rustfmt failures
gate_formatrancargo fmt --checkand then prettier, so the function's exit status was only ever prettier's. rustfmt failures were swallowed — which is whycrates/toolpath-codex/src/{io.rs,paths.rs}have been unformatted onmainwithout anyone hearing about it. They are reformatted here (purecargo fmt --alloutput, no other edits).Both checks now always run — one pass reports every formatting problem — and either failing fails the gate. Verified by appending a deliberately misformatted function and running both versions of the gate: the old one printed
PASS: format, the new one prints the rustfmt diff, still runs prettier, thenFAIL: format.While in there:
gate_format's barecd "${_root}/site"leaked the working directory into every gate that ran after it (gates execute in the caller's shell). It andgate_site'scdnow run in subshells. The other gates were checked for the same masking pattern —gate_examplesalready accumulates failures, the rest are single-command — andscripts/{fmt,check}.shuseset -euo pipefail.Testing
All 8 quality gates green in a clean clone: format, shellcheck, clippy, test, doc, examples, plugin, site.
Known adjacent issue left alone to keep this diff to the two fixes:
wasm_is_stale()passes.cargo/config.tomltofindunder a-name '*.rs' -o -name 'Cargo.toml'predicate that can never match it, so--if-changedmisses link-flag changes.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.