ci: track stable in rust-toolchain.toml, enforce MSRV in its own job

.github/workflows/test.yml

@@ -48,6 +48,70 @@ jobs:
               - '**'
               - '!**.md'
 
+  # Everything else in CI runs on `stable` (see rust-toolchain.toml), so the MSRV
+  # promised to downstream users needs its own gate. This job is the only place
+  # the MSRV toolchain is used; it reads the version from the workspace
+  # Cargo.toml so `rust-version` stays the single source of truth.
+  #
+  # Scope: build only, no `--all-targets`. The promise is that consumers can
+  # *build* the published crates with the MSRV, not that our test suite runs
+  # there — dev-dependencies are free to require a newer compiler.
+  #
+  # Linux-only: MSRV regressions almost always come from a dependency raising its
+  # own `rust-version`, which is platform-independent. Running the full OS matrix
+  # would triple the cost to catch only platform-gated regressions (e.g. a
+  # windows-sys bump), which the stable `test` matrix would surface anyway once
+  # the dependency reaches a release we build.
+  msrv:
+    name: MSRV build
+    needs: changes
+    if: needs.changes.outputs.src == 'true'
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+
+      - name: Read MSRV from Cargo.toml
+        id: msrv
+        run: |
+          set -euo pipefail
+          version=$(awk -F'"' '/^rust-version *=/ { print $2; exit }' Cargo.toml)
+          if [ -z "$version" ]; then
+            echo "::error file=Cargo.toml::could not parse rust-version"
+            exit 1
+          fi
+          echo "Detected MSRV: $version"
+          echo "version=$version" >> "$GITHUB_OUTPUT"
+
+      - name: Remove the runner's bundled Rust toolchain
+        run: rustup toolchain remove stable 2>/dev/null || true
+
+      - uses: actions-rust-lang/setup-rust-toolchain@166cdcfd11aee3cb47222f9ddb555ce30ddb9659 # v1.17.0
+        with:
+          toolchain: ${{ steps.msrv.outputs.version }}
+          target: wasm32-unknown-unknown
+          cache-shared-key: ${{ runner.os }}-msrv
+          cache-bin: false
+          rustflags: ""
+
+      # ref-tests and ic-utils-bindgen-tests are the only `publish = false`
+      # members; they depend on pocket-ic from the IC monorepo, whose MSRV runs
+      # far ahead of ours. Excluding (rather than listing the published crates)
+      # keeps a newly added published crate covered by default.
+      - name: Build with MSRV
+        run: |
+          cargo build --locked --workspace \
+            --exclude ref-tests --exclude ic-utils-bindgen-tests
+          cargo build --locked --workspace \
+            --exclude ref-tests --exclude ic-utils-bindgen-tests --all-features
+
+      # Browser consumers build ic-agent for wasm at the MSRV too; mirrors the
+      # WASM step in lint.yml.
+      - name: Build with MSRV (WASM)
+        run: |
+          CARGO_TARGET_DIR=target/wasm cargo build --locked --target wasm32-unknown-unknown \
+            -p ic-agent --features wasm-bindgen -p ic-utils
+
   # Workspace tests for every crate except ref-tests, on all three OSes. Because
   # each crate is tested from its own directory, the heavy pocket-ic dependency
   # (only used by ref-tests) never compiles here, keeping this job's cache small.
@@ -199,17 +263,27 @@ jobs:
 
       # CARGO_TARGET_DIR=target/wasm keeps wasm artifacts under ./target, so
       # rust-cache (which caches ./target) still picks them up.
+      #
+      # --lib restricts this to the lib target's #[wasm_bindgen_test] tests. Newer
+      # toolchains also run doctests for wasm targets (1.88 skipped them), and
+      # ic-agent's doctests cannot compile there: they use #[tokio::main], and
+      # tokio is deliberately a dev-dependency only under
+      # cfg(not(target_family = "wasm")). Doctests are covered on the host by the
+      # `test` job above; running them inside a headless browser adds nothing.
       - name: Run Tests (WASM)
-        run: CARGO_TARGET_DIR=target/wasm wasm-pack test --chrome --headless ic-agent --features wasm-bindgen
+        run: CARGO_TARGET_DIR=target/wasm wasm-pack test --chrome --headless ic-agent --features wasm-bindgen --lib
 
   aggregate:
     name: test:required
     # Runs only when the test jobs ran; on docs-only changes this skips, and a
     # skipped required check counts as passing for branch protection.
     if: always() && needs.changes.outputs.src == 'true'
     runs-on: ubuntu-latest
-    needs: [changes, test, ref_tests, wasm]
+    needs: [changes, msrv, test, ref_tests, wasm]
     steps:
+      - name: Check MSRV result
+        if: ${{ needs.msrv.result != 'success' }}
+        run: exit 1
       - name: Check test result
         if: ${{ needs.test.result != 'success' }}
         run: exit 1

Cargo.toml

@@ -17,9 +17,16 @@ version = "0.49.2"
 authors = ["DFINITY Stiftung <sdk@dfinity.org>"]
 edition = "2021"
 repository = "https://github.com/dfinity/agent-rs"
-# MSRV
-# Avoid updating this field unless we use new Rust features
-# Sync rust-version in rust-toolchain.toml
+# MSRV — the single source of truth. The `msrv` job in
+# .github/workflows/test.yml parses this line to pick its toolchain, so bumping
+# the MSRV is a one-line change here.
+#
+# Avoid updating this field unless we need new Rust features, or a dependency
+# forces it. It is a compatibility promise to downstream users: raise it only in
+# a minor (not patch) release, and note it in CHANGELOG.md.
+#
+# rust-toolchain.toml is intentionally NOT pinned to this version; it tracks
+# stable.
 rust-version = "1.88.0"
 license = "Apache-2.0"
 

README.md

@@ -13,6 +13,31 @@ We use `cargo` to build this repo. Make sure you have rust stable installed. To
 cargo build
 ```
 
+### Rust toolchain and MSRV
+Two different Rust versions are in play, and they are deliberately decoupled:
+
+| | Where | Value |
+|---|---|---|
+| Development toolchain | `rust-toolchain.toml` | `stable` |
+| MSRV | `rust-version` in the workspace `Cargo.toml` | pinned |
+
+Development, `cargo fmt`, `cargo clippy` and all CI jobs except one use the
+latest **stable** release. The **MSRV** is the oldest compiler the published
+crates are guaranteed to build with, and is enforced by the `msrv` job in
+[`.github/workflows/test.yml`](.github/workflows/test.yml), which reads the
+version out of `Cargo.toml`. That job builds the published crates only — the
+MSRV covers building the libraries, not running our test suite, so
+dev-dependencies may require a newer compiler.
+
+Consequences worth knowing:
+
+* Bumping the MSRV is a one-line change to `rust-version`. Do it only when we
+  need newer Rust features or a dependency forces it, in a minor (not patch)
+  release, with a CHANGELOG entry.
+* Because clippy tracks stable, a new Rust release can introduce lints that fail
+  CI on an otherwise untouched branch. The fix is a small lint-cleanup PR, not an
+  MSRV or toolchain change.
+
 ## Testing
 There are two suites of tests that can be executed from this repo; the regular cargo tests and
 the ic-ref tests. In order to run the ic-ref tests, you will need a running local reference

ic-agent/http_mock_service_worker.js

@@ -38,10 +38,31 @@ async function getMock(nonce) {
     });
 }
 
+// Handle one request at a time.
+//
+// Both the `hits` counter and the route list are read-modify-write cycles over a
+// single IndexedDB record: `getMock` reads the whole record in one transaction,
+// the handler mutates its copy, and `setMock` writes the whole record back in
+// another. Two requests in flight against the same mock therefore both read the
+// pre-state and the second write silently discards the first one's mutation.
+//
+// That is not hypothetical: `Agent::query` issues its `query` and its
+// `read_state` concurrently via `try_join!`, so a certifying-agent test reliably
+// has two overlapping requests and can lose one of the two hit increments. There
+// is nothing to gain from serving these concurrently — the responses come from
+// canned data — so serialize the handlers and keep each cycle atomic.
+let tail = Promise.resolve();
+function serialized(fn) {
+    const run = tail.then(fn);
+    // Keep the chain alive regardless of how this handler settles.
+    tail = run.then(() => {}, () => {});
+    return run;
+}
+
 // Status codes are chosen to avoid being picked up as successes by tests expecting a 404 or 500.
 
 self.addEventListener("fetch", (event) => {
-    event.respondWith((async () => {
+    event.respondWith(serialized(async () => {
         try {
             const request = event.request;
             const url = new URL(request.url);
@@ -83,7 +104,7 @@ self.addEventListener("fetch", (event) => {
         } catch (e) {
             return new Response(e.toString(), { status: 503 });
         }
-    })())
+    }));
 });
 
 self.addEventListener("activate", (event) => {

ic-agent/src/agent/agent_test.rs

@@ -21,7 +21,7 @@
     builder.with_verify_query_signatures(false).build().unwrap()
 }
 
 fn make_agent_with_route_provider(
     route_provider: Arc<dyn RouteProvider>,
     tcp_retries: usize,
 ) -> Agent {
@@ -314,7 +314,7 @@
 }
 
 #[cfg_attr(not(target_family = "wasm"), tokio::test)]
 async fn reqwest_client_status_okay_when_request_retried() -> Result<(), AgentError> {
     let map = BTreeMap::new();
     let response = serde_cbor::Value::Map(map);
     let (read_mock, url) = mock(
@@ -878,7 +878,10 @@
 
     pub async fn assert_mock(nonce: String) {
         let hits = get_hits(&nonce).await;
-        assert!(hits.values().all(|x| *x > 0));
+        assert!(
+            hits.values().all(|x| *x > 0),
+            "some mocked routes were never hit: {hits:?}"
+        );
     }
 
     pub async fn assert_single_mock(method: &str, path: &str, nonce: &String) {

rust-toolchain.toml

@@ -1,7 +1,14 @@
 [toolchain]
-# MSRV
-# Avoid updating this field unless we use new Rust features
-# Sync rust-version in workspace Cargo.toml
-channel = "1.88.0"
+# Development toolchain — NOT the MSRV.
+#
+# We deliberately track the latest stable release here so that local builds,
+# rust-analyzer, clippy and rustfmt all use a current compiler. Pinning this to
+# the MSRV instead makes the repo progressively harder to work in as the MSRV
+# ages (rust-analyzer refuses to work with sufficiently old toolchains) and
+# hides new clippy/rustc diagnostics until the MSRV is bumped.
+#
+# The MSRV is `rust-version` in the workspace Cargo.toml, and is enforced by the
+# `msrv` job in .github/workflows/test.yml, which reads it from there.
+channel = "stable"
 components = ["rustfmt", "clippy"]
 targets = ["wasm32-unknown-unknown"]