Repository navigation
Conversation
…r-js deps trusted-server-core depends on trusted-server-js, whose build script runs the TSJS build with the npm on PATH. Most Rust CI jobs never set up the .tool-versions Node, so they built the embedded bundles with the runner image Node (22 on ubuntu-24.04). Add setup-node with node-version-file and npm caching to test-cloudflare, test-spin, test-parity and format-rust, and to the shared integration-test composite action before its Rust builds. Drop the now-redundant setup-node step from the integration-tests job. Remove the unused direct trusted-server-js dependency from the Cloudflare and Spin adapters; both still get it through trusted-server-core. Closes #1204 Signed-off-by: dhruv8sh <dhruv8sh@proton.me>
ChristianPavilonis
approved these changes
Oct 8, 2026
ChristianPavilonis
left a comment
Collaborator
There was a problem hiding this comment.
Review summary
Reviewed 2b1914286d53d01a768850c237b7274865b24037 against 7a0ecb4cbfa39af3d83f7e1ed2733d7eabdc4cb1. All seven changed files were inspected, including CI callers and the adapter-to-core-to-JS dependency and serving paths. No actionable issues introduced by this PR were found.
Safety proof
- Logs from all eight affected CI jobs confirmed Node
v24.12.0before their first TSJS build. Wrangler installation and the browser job's separate two-lockfile cache also succeeded. - Locked, offline dependency checks confirmed both adapters retain JS through core on native and production WASM targets. Full manifest and lockfile comparison found only the two intended dependency-edge removals.
Validation and review context
- Local
cargo fmt --all -- --check, diff whitespace checks, actionlint with ShellCheck, locked/offline Cargo metadata and dependency-tree checks, and in-memory manifest, lockfile, consumer, and CI-log assertions passed. - All reported PR checks passed. Inspected logs confirmed 1,185 JS tests, native adapter tests, production-target compilation, parity, integration, and browser tests passed. CI merge commit
0002055156cf3acb3eaa604fc045ced11f0adb7chas the same tree as the reviewed head. - Existing reviews, inline comments, issue comments, and review threads contained no feedback.
- Manual deployment was not exercised. Build and runtime evidence was reused from verified same-tree CI rather than rerun locally. Review performed without delegation or file edits; worktree remained clean.
This branch has not been deployed
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.
Summary
trusted-server-coredepends ontrusted-server-js, whose build script runsnpm ci/npm run buildwith thenpmon PATH and embeds the bundles. Most Rust CI jobs never set up the.tool-versionsNode, so they built those bundles with the runner image's Node (22 onubuntu-24.04) instead of the pinned 24.12.0. They would also have missed the planned pin bump (Upgrade to Rust 1.98.1 and align dependencies with EdgeZero #1123).trusted-server-jsdependency from the Cloudflare and Spin adapters, as Remove unused dependencies and disable unnecessary test targets #614 did for Fastly. Both still get it through core.Changes
.github/workflows/test.ymlsetup-node(node-version-file: .tool-versions,cache: npm) before cargo intest-cloudflare,test-spin,test-parity.github/workflows/format.ymlcargo fmt/ clippy informat-rust.github/actions/setup-integration-test-env/action.ymlprepare-artifacts,integration-tests-fastly-ec,browser-tests.github/workflows/integration-tests.ymlintegration-testsjob'ssetup-nodestep, now done by the composite actioncrates/trusted-server-adapter-cloudflare/Cargo.tomltrusted-server-jsdependencycrates/trusted-server-adapter-spin/Cargo.tomltrusted-server-jsdependencyCargo.lockNotes:
mainalready set up Node before cargo intest-axumandtest-cli, so those jobs are unchanged.browser-testskeeps its ownsetup-nodestep for its two-lockfile npm cache.integration-tests-fastly-ec/browser-tests, and enforcing the Node version locally (enginesfield or abuild.rscheck).Closes
Closes #1204
Test plan
cargo test-fastly && cargo test-axumcargo clippy-fastly && cargo clippy-axumcargo fmt --all -- --checkcd crates/trusted-server-js/lib && npx vitest runcd crates/trusted-server-js/lib && npm run formatcd docs && npm run formatcargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1fastly compute servecargo check-cloudflare,cargo check-spin,cargo test-cloudflare,cargo test-spin, and clippy for Cloudflare (native + wasm) and Spin (native + wasm). A script also checked that every job'ssetup-nodestep comes before its first cargo step that compiles core.Vitest passes 1185/1185 under the pinned Node 24.12.0. Under Node 26, 27 Permutive/Sourcepoint tests fail because Node 26's built-in
localStoragehides jsdom's. That is local version drift, not caused by this PR. To confirm the CI change, check that each touched job's setup step shows the.tool-versionsNode.Checklist
unwrap()in production code — useexpect("should ...")(no Rust code changed)tracingmacros (notprintln!) (no Rust code changed)