Repairs from the review of #759: whole-token coverage, a classifier that falls back to the whole CI, a concurrency group per push to main, cache keys that name the install list, a step for the shard's installs, and a record that leaves a tab-holding root unrecorded

Author: speak-agent

.agents/docs/2026-10-02-pr-ci-acceleration-and-the-toolchain-specification-design.md

@@ -421,8 +421,13 @@ repository has paid for that before.
   are the main cause of F3. They return only if the build becomes incremental across checkouts. That needs
   a measured cause (Part VII) and a fix that keeps the stale-object hazards already recorded in
   `.agents/docs/2026-05-15-stdcompat-restat-e2e.md` and `cross-build-test.yml:416-434` out of the result.
-- **Concurrency.** `cancel-in-progress` becomes `${{ github.event_name == 'pull_request' }}`. A push to
-  `main` runs to completion and writes its caches; a superseded pull-request run is still cancelled.
+- **Concurrency.** `cancel-in-progress` becomes `${{ github.event_name == 'pull_request' }}`, and a push
+  to `main` has a group of its own commit. A push to `main` runs to completion and writes its caches; a
+  superseded pull-request run is still cancelled. One group for all pushes to `main` would not be enough,
+  because GitHub keeps one pending run per group and cancels the older pending run when another arrives.
+- **Keys name what fills the cache.** The sandbox key hashes `ci.yml` as well as `mcpp.toml` and
+  `.xlings.json`, because `ci.yml` lists what the build job installs before it saves. A cache saved under
+  an unchanged key is never saved again, so a toolchain added to that list would otherwise never reach it.
 - **Wine.** The Wine packages come from one pinned archive, published once as a release asset of this
   repository, instead of from the `apt` mirrors with a cache that eviction removes (F7). The archive's
   sha256 is checked before installation.
@@ -620,6 +625,18 @@ reading taken while building it.
   machine that has them and failed on every runner, and nothing noticed, because
   no runner ran it. The fixture's root is now a library, which is what the test
   says it is: "this test compiles only".
+- **The review of the change** (a code review of mcpp#759) found eight defects. Each was repaired before
+  merging:
+  - the coverage check matched test names inside comments and inside longer names, and now matches whole
+    tokens outside comments;
+  - `changes` failed, rather than running the whole CI, when the GitHub API did not answer;
+  - pushes to `main` shared a concurrency group;
+  - the cache keys did not name the install list;
+  - the Linux shards' toolchain installs shared the suite's time limit, and now have a step of their own;
+  - the `macos-27` label's effect was undocumented (it is read on the next push);
+  - the CHANGELOG gave three Linux shards;
+  - a path-dependency root with a tab in its path could be cut short in the build record, and is now left
+    unrecorded, which declines the fast path.
 - **Coverage found more than F9.** Classifying the tests of the 2026-10-01
   logs found 24 that ran on no runner and were named by no workflow. The
   seven `llvm` tests are among them, and so are three `musl` tests: the probe

.github/actions/bootstrap-mcpp/action.yml

@@ -85,7 +85,10 @@ runs:
         # sandbox — which is what actually resolves dependencies — would
         # silently stay behind (observed: a 0.4.30 sandbox surviving under a
         # 0.4.69 bootstrap for weeks).
-        key: mcpp-sandbox-${{ runner.os }}-${{ runner.arch }}-ci-xl${{ inputs.xlings-version }}-${{ hashFiles('mcpp.toml', '.xlings.json') }}
+        # ci.yml is part of the key because it names the toolchains the build
+        # job installs before it saves (`prewarm`): a sandbox saved under an
+        # unchanged key would never gain one added there.
+        key: mcpp-sandbox-${{ runner.os }}-${{ runner.arch }}-ci-xl${{ inputs.xlings-version }}-${{ hashFiles('mcpp.toml', '.xlings.json', '.github/workflows/ci.yml') }}
         restore-keys: |
           mcpp-sandbox-${{ runner.os }}-${{ runner.arch }}-ci-xl${{ inputs.xlings-version }}-
 

.github/tools/check_e2e_coverage.py

@@ -18,9 +18,11 @@
 under tests/e2e:
 
   ran        some report says pass, fail or timeout;
-  job        no report ran it, but a workflow names it (by file name, or by its
-             number as an `E2E_ONLY` pattern such as `239_*.sh`): a dedicated job
-             runs it and asserts its result itself;
+  job        no report ran it, but a workflow names it outside a comment, as a
+             whole token: by file name, by name without `.sh`, or by its number
+             as an `E2E_ONLY` pattern such as `239_*.sh`. A dedicated job runs it
+             and asserts its result itself. A name in a comment does not count,
+             and neither does a longer name that contains it;
   excused    tests/e2e/coverage-exceptions.tsv lists it with the reason no hosted
              runner can run it;
   uncovered  none of these. The check fails.
@@ -74,9 +76,25 @@ def read_exceptions(path: Path) -> dict[str, str]:
     return out
 
 
-def named_by_a_workflow(test: str, workflows: str) -> bool:
-    number = test.split("_", 1)[0]
-    return test in workflows or test[:-3] in workflows or f"{number}_*" in workflows
+TOKEN = re.compile(r"(?<![A-Za-z0-9_.-])(\d+[a-z]?_(?:\*|[A-Za-z0-9_]+))(?:\.sh)?(?![A-Za-z0-9_])")
+
+
+def workflow_tokens(texts: list[str]) -> set[str]:
+    """Every test name or `<number>_*` pattern named outside a comment."""
+    tokens: set[str] = set()
+    for text in texts:
+        for line in text.splitlines():
+            code = line.split("#", 1)[0] if line.lstrip().startswith("#") else line
+            if not code.strip():
+                continue
+            tokens.update(m.group(1) for m in TOKEN.finditer(code))
+    return tokens
+
+
+def named_by_a_workflow(test: str, tokens: set[str]) -> bool:
+    stem = test[:-3]
+    number = stem.split("_", 1)[0]
+    return stem in tokens or f"{number}_*" in tokens
 
 
 def main() -> int:
@@ -88,8 +106,8 @@ def main() -> int:
     root = Path(args.root).resolve()
 
     tests = sorted(p.name for p in (root / "tests" / "e2e").glob("[0-9]*.sh"))
-    workflows = "\n".join(p.read_text(encoding="utf-8")
-                          for p in sorted((root / ".github" / "workflows").glob("*.yml")))
+    workflows = workflow_tokens([p.read_text(encoding="utf-8")
+                                 for p in sorted((root / ".github" / "workflows").glob("*.yml"))])
     exceptions = read_exceptions(root / "tests" / "e2e" / "coverage-exceptions.tsv")
 
     ran: dict[str, set[str]] = defaultdict(set)

.github/workflows/ci-linux-e2e.yml

@@ -48,23 +48,12 @@ jobs:
         with:
           host: linux-x86_64
 
-      - name: E2E suite
-        # About twice the shard's budget of ten minutes (R4): reached only by a
-        # hang. The per-test 600 s limit in run_all.sh names the test that hung.
-        timeout-minutes: 22
+      # Its own step and its own limit, so that downloads on a cold sandbox are
+      # not taken from the suite's budget.
+      - name: The toolchains the suite probes
+        timeout-minutes: 15
         run: |
           set -euo pipefail
-          # MCPP is this commit's binary and MCPP_BOOT the released bootstrap
-          # (use-built-mcpp). e2e 252 needs the latter: the claim that an older
-          # client can build against a package this commit produces is only
-          # worth making against a real old binary.
-          export MCPP MCPP_BOOT
-          test -x "$MCPP_VENDORED_XLINGS"
-          # GitHub-hosted runners are outside CN; keep CI toolchain downloads on
-          # the global mirror while mcpp's default remains CN for fresh local
-          # sandboxes. E2E tests with their own MCPP_HOME read this variable.
-          export MCPP_E2E_TOOLCHAIN_MIRROR=GLOBAL
-          "$MCPP" self config
           # Pin the global default so test 28 (default-toolchain path) gets a
           # deterministic GNU answer instead of an auto-install pick. Installed
           # explicitly, not assumed: the restored sandbox is the build job's,
@@ -81,6 +70,24 @@ jobs:
           "$MCPP" toolchain install llvm 22.1.8
           "$MCPP" toolchain install mingw-cross 16.1.0
           XLINGS_HOME="$MCPP_HOME/registry" "$MCPP_VENDORED_XLINGS" install xim:nasm -y
+
+      - name: E2E suite
+        # About twice the shard's budget of ten minutes (R4): reached only by a
+        # hang. The per-test 600 s limit in run_all.sh names the test that hung.
+        timeout-minutes: 22
+        run: |
+          set -euo pipefail
+          # MCPP is this commit's binary and MCPP_BOOT the released bootstrap
+          # (use-built-mcpp). e2e 252 needs the latter: the claim that an older
+          # client can build against a package this commit produces is only
+          # worth making against a real old binary.
+          export MCPP MCPP_BOOT
+          test -x "$MCPP_VENDORED_XLINGS"
+          # GitHub-hosted runners are outside CN; keep CI toolchain downloads on
+          # the global mirror while mcpp's default remains CN for fresh local
+          # sandboxes. E2E tests with their own MCPP_HOME read this variable.
+          export MCPP_E2E_TOOLCHAIN_MIRROR=GLOBAL
+          "$MCPP" self config
           bash tests/e2e/run_all.sh
 
       # One file, no glob: the report the e2e-coverage job of ci.yml reads.
@@ -346,7 +353,7 @@ jobs:
         uses: actions/cache/restore@v4
         with:
           path: ~/.mcpp
-          key: mcpp-hermetic-${{ hashFiles('mcpp.toml') }}
+          key: mcpp-hermetic-${{ hashFiles('mcpp.toml', '.github/workflows/ci-linux-e2e.yml') }}
           restore-keys: |
             mcpp-hermetic-
 

.github/workflows/ci.yml

@@ -27,11 +27,13 @@ on:
     branches: [ main ]
   workflow_dispatch:
 
-# A superseded pull-request run is cancelled. A push to main runs to the end,
-# because its build jobs are the only writers of the caches (R3), and a
-# cancelled run saves nothing.
+# A superseded pull-request run is cancelled. Every push to main runs to the
+# end, in a group of its own commit: its build jobs are the only writers of the
+# caches (R3), and a cancelled run saves nothing. A group shared by all pushes
+# to main would still lose runs, because GitHub keeps one pending run per group
+# and cancels the older pending one when a third arrives.
 concurrency:
-  group: ci-${{ github.ref }}
+  group: ci-${{ github.event_name == 'push' && github.sha || github.ref }}
   cancel-in-progress: ${{ github.event_name == 'pull_request' }}
 
 permissions:
@@ -61,15 +63,19 @@ jobs:
         run: |
           set -euo pipefail
           full() { echo "code=true" >> "$GITHUB_OUTPUT"; echo "code=true ($1)"; }
+          # Anything this step cannot list runs the whole CI: an API error, a
+          # force-push whose previous commit is gone, an event without a diff.
           case "$EVENT" in
             pull_request)
               gh api --paginate "repos/$REPO/pulls/$PR/files" \
-                --jq '.[] | .filename, (.previous_filename // empty)' > changed.txt ;;
+                --jq '.[] | .filename, (.previous_filename // empty)' > changed.txt \
+                || { full "the files of the pull request could not be listed"; exit 0; } ;;
             push)
               if [ -z "$BEFORE" ] || [ "$BEFORE" = 0000000000000000000000000000000000000000 ]; then
                 full "a push with no previous commit"; exit 0
               fi
-              gh api "repos/$REPO/compare/$BEFORE...$SHA" > compare.json
+              gh api "repos/$REPO/compare/$BEFORE...$SHA" > compare.json \
+                || { full "the push could not be compared with its previous commit"; exit 0; }
               # The compare API lists at most 300 files; a longer list is
               # treated as a change of everything.
               if [ "$(jq '.files | length' compare.json)" -ge 300 ]; then
@@ -187,7 +193,10 @@ jobs:
     uses: ./.github/workflows/ci-macos.yml
     with:
       # R7: the known-red legs (#669) run where they can change a decision: on
-      # main, on dispatch, and on a pull request labelled `macos-27`.
+      # main, on dispatch, and on a pull request labelled `macos-27`. The label
+      # is read when the pull request is pushed to; adding it starts no run,
+      # because a run on every label of every pull request would cost a whole
+      # CI each time.
       known-red: ${{ github.event_name != 'pull_request' || contains(github.event.pull_request.labels.*.name, 'macos-27') }}
 
   macos-e2e:

.github/workflows/cross-build-test.yml

@@ -80,7 +80,7 @@ jobs:
         uses: actions/cache/restore@v4
         with:
           path: ~/.mcpp
-          key: mcpp-sandbox-${{ runner.os }}-cross-${{ matrix.target }}-${{ hashFiles('mcpp.toml', '.xlings.json') }}
+          key: mcpp-sandbox-${{ runner.os }}-cross-${{ matrix.target }}-${{ hashFiles('mcpp.toml', '.xlings.json', '.github/workflows/cross-build-test.yml') }}
           restore-keys: |
             mcpp-sandbox-${{ runner.os }}-cross-${{ matrix.target }}-
 
@@ -244,7 +244,7 @@ jobs:
         uses: actions/cache/restore@v4
         with:
           path: ~/.mcpp
-          key: mcpp-sandbox-${{ runner.os }}-mingw-cross-${{ hashFiles('mcpp.toml', '.xlings.json') }}
+          key: mcpp-sandbox-${{ runner.os }}-mingw-cross-${{ hashFiles('mcpp.toml', '.xlings.json', '.github/workflows/cross-build-test.yml') }}
           restore-keys: |
             mcpp-sandbox-${{ runner.os }}-mingw-cross-
 

CHANGELOG.md

@@ -57,7 +57,7 @@ No default toolchain changes.
 - **One writer per cache.** Every job restores. One job per key saves, on a
   push to `main` only, and `target/` is no longer cached, because a restored
   `target/` made no build incremental.
-- **E2E shards by measured duration**, three on Linux, three on Windows and two
+- **E2E shards by measured duration**, four on Linux, three on Windows and two
   on macOS, from `tests/e2e/timings/`. `run_all.sh` takes `E2E_TIMINGS`,
   `E2E_REPORT` and `E2E_LIST`.
 - **Every e2e test runs somewhere.** The `e2e-coverage` job fails when a test
@@ -66,7 +66,7 @@ No default toolchain changes.
   `run_all.sh` grants `llvm` on Linux, and it probes `musl` and `mingw-cross`
   by family rather than by one release.
 - The legs that are known red (#669) run on `main`, on dispatch, and on a pull
-  request labelled `macos-27`.
+  request labelled `macos-27` (the label is read on the next push).
 
 ## [2026.10.1.3] - 2026-10-01
 

src/build/execute.cppm

@@ -667,10 +667,21 @@ void write_build_cache_entries(const std::filesystem::path& path,
               << '\n';
         f << "profile=" << e.profile << '\n';
         f << "cacheMode=" << e.cacheMode << '\n';
-        f << "depSources=" << e.depSourceRoots.size() << '\n';
-        for (auto& r : e.depSourceRoots)
-            f << r.root.generic_string() << '\t' << join_extensions(r.moduleExtensions)
-              << '\t' << join_extensions(r.deviceExtensions) << '\n';
+        // A root whose path holds a tab or a line break cannot be written in
+        // this line format: read back, the path would be cut at the tab, and a
+        // directory that happened to exist under the shorter name would be
+        // swept with the wrong table. Such an entry records no roots, which
+        // reads as "predates the list" and declines the fast path, the safe
+        // direction.
+        const bool writable = std::ranges::none_of(e.depSourceRoots, [](auto const& r) {
+            return r.root.generic_string().find_first_of("\t\n\r") != std::string::npos;
+        });
+        if (writable) {
+            f << "depSources=" << e.depSourceRoots.size() << '\n';
+            for (auto& r : e.depSourceRoots)
+                f << r.root.generic_string() << '\t' << join_extensions(r.moduleExtensions)
+                  << '\t' << join_extensions(r.deviceExtensions) << '\n';
+        }
         f << "runner=" << (e.runnerDeclared ? 1 : 0) << '\n';
         f << "runtier=" << (e.runTierPending ? 1 : 0) << '\n';
         f << "features=" << e.features << '\n';

tests/scripts/test_check_e2e_coverage.py

@@ -48,6 +48,21 @@ def test_ran_named_and_excused_tests_are_covered(self) -> None:
         self.assertEqual(r.returncode, 0, r.stdout)
         self.assertIn("1 ran on a shard, 2 run by a dedicated job, 1 excused, 0 uncovered", r.stdout)
 
+    def test_a_name_in_a_comment_or_inside_a_longer_name_does_not_count(self) -> None:
+        e2e = self.root / "tests" / "e2e"
+        (e2e / "60_commented.sh").write_text("# x\n# requires: llvm\n")
+        (e2e / "70_short.sh").write_text("# x\n# requires: llvm\n")
+        with (self.root / ".github" / "workflows" / "ci.yml").open("a") as f:
+            f.write("      # 60_commented.sh is mentioned in a comment only\n"
+                    "run: bash tests/e2e/170_short_but_longer.sh\n")
+        (self.reports / "e2e-report-linux-2.tsv").write_text(
+            "skip\t60_commented.sh\t0\tmissing capability: llvm\n"
+            "skip\t70_short.sh\t0\tmissing capability: llvm\n")
+        r = self.run_check()
+        self.assertEqual(r.returncode, 1)
+        self.assertIn("UNCOVERED: 60_commented.sh", r.stdout)
+        self.assertIn("UNCOVERED: 70_short.sh", r.stdout)
+
     def test_a_test_that_runs_nowhere_fails(self) -> None:
         (self.root / "tests" / "e2e" / "50_nowhere.sh").write_text("# x\n# requires: llvm\n")
         (self.reports / "e2e-report-linux-2.tsv").write_text(

tests/unit/test_build_cache_record.cpp

@@ -150,6 +150,23 @@ TEST(BuildCacheRecord, TheEngineAndEachRootsTablesSurviveAWriteAndARead) {
     EXPECT_EQ(read[0].toolchainRequest, e.toolchainRequest);
 }
 
+// A path the line format cannot hold is not written at all, so the entry reads
+// as one that predates the list and declines, rather than as a shorter path.
+TEST(BuildCacheRecord, ARootWithATabInItsPathIsLeftUnrecorded) {
+    Tmp tmp;
+    auto e = minimal_entry();
+    e.depSourceRoots = {{"/work/a\tb", {".ixx"}, {}}};
+    e.depSourceRootsRecorded = true;
+    e.toolchainRecorded = true;
+    e.toolchainRequest = "cli=;default=gcc@16.1.0";
+    write_build_cache_entries(tmp.path / "target" / ".build_cache", {e});
+    const auto read = read_build_cache(tmp.path);
+    ASSERT_EQ(read.size(), 1u);
+    EXPECT_FALSE(read[0].depSourceRootsRecorded);
+    EXPECT_TRUE(read[0].depSourceRoots.empty());
+    EXPECT_TRUE(read[0].toolchainRecorded);
+}
+
 TEST(BuildCacheRecord, AnEmptyListOfRootsIsRecordedNotAbsent) {
     Tmp tmp;
     auto e = minimal_entry();