From aad8e98e06b403e6b6538c6cb5c7cfda82f85459 Mon Sep 17 00:00:00 2001 From: Bohan-J Date: Mon, 27 Jul 2026 15:14:09 +0800 Subject: [PATCH] fix(ci): scope turbo cache to the resolved runtime, drop typecheck from tests (MUL-5347) Addresses review feedback on the caching commit. Must-fix: the cache was not isolated by interpreter. `node-version: 22` floats across patch releases and turbo's global hash does not include the interpreter at all (`engines` is null in its dry-run cache inputs), so a runner moving to another 22.x would leave the workflow text and every task hash unchanged, restore the old cache through `restore-keys` and report green without executing anything. Both jobs now resolve `node --version` into a step output and carry it, plus `runner.arch`, in the cache key and the restore prefix. Must-fix: `actions/cache@v4` runs on the deprecated Node 20 runtime and was being force-migrated to Node 24 with a warning on every run. Moved to v6 -- the review suggested v5, but v6.1.0 is current and satisfies the same runner floor (>= 2.327.1; hosted runners are on 2.335.1). `test` no longer depends on `^typecheck`. It needs dependency sources in its hash, not a type check: a new hash-only `cache-inputs` transit task carries them. No package implements that script, so every node resolves to and nothing executes, while the edges still pull each dependency's files into the hash. Declared recursively so the chain survives a package gaining a purely transitive dependency. This returns the ~223 CPU-seconds the previous commit gave up: the cold test job goes from 7 tasks / 115.7s back to 4 tasks / 67.2s, and editing packages/ui still correctly re-runs the views, web and desktop suites while core:test stays cached. Corrects a comment that claimed `^test` would miss packages/ui because it has no test script. It would not -- turbo materialises a node that participates in hashing, verified against the dry graph. `^test` is still the wrong edge here, but because it serialises the suites behind each other, not for the stated reason. Also drops the stale note that tests no longer depend on `^typecheck`, and swaps the aggregate gate's `always()` for `!cancelled()` so a run superseded by a newer push does not spend a runner on a verdict nobody reads. Co-authored-by: multica-agent --- .github/workflows/ci.yml | 37 ++++++++++++++++++++++++++++--------- turbo.json | 24 ++++++++++++++++-------- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ab14194069..26351b9c24 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -113,6 +113,16 @@ jobs: if: ${{ needs.changes.outputs.frontend == 'true' }} run: pnpm install + # `node-version: 22` above floats across patch releases, and turbo's + # global hash does not include the interpreter at all (`engines` is null + # in its dry-run cache inputs). Without the resolved version in the key, + # a runner silently moving to another 22.x would restore a cache built by + # the old interpreter and report green without executing anything. + - name: Resolve runtime for cache key + id: runtime + if: ${{ needs.changes.outputs.frontend == 'true' }} + run: echo "node=$(node --version)" >> "$GITHUB_OUTPUT" + # Cache entries are immutable, so the key carries the commit SHA to make # every run publish a fresh one and `restore-keys` falls back to the most # recent prefix match. The two frontend jobs run disjoint task sets, so @@ -121,12 +131,12 @@ jobs: # tasks hit on the first push) and writes its own (so re-pushes hit too). - name: Restore turbo cache if: ${{ needs.changes.outputs.frontend == 'true' }} - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: .turbo/cache - key: turbo-build-${{ runner.os }}-${{ github.sha }} + key: turbo-build-${{ runner.os }}-${{ runner.arch }}-${{ steps.runtime.outputs.node }}-${{ github.sha }} restore-keys: | - turbo-build-${{ runner.os }}- + turbo-build-${{ runner.os }}-${{ runner.arch }}-${{ steps.runtime.outputs.node }}- - name: Test self-host env derivation if: ${{ needs.changes.outputs.frontend == 'true' }} @@ -176,24 +186,30 @@ jobs: if: ${{ needs.changes.outputs.frontend == 'true' }} run: pnpm install + - name: Resolve runtime for cache key + id: runtime + if: ${{ needs.changes.outputs.frontend == 'true' }} + run: echo "node=$(node --version)" >> "$GITHUB_OUTPUT" + # See frontend-build for the key strategy. These entries are tiny (~60KB # measured): `test` declares no outputs, so turbo caches exit codes and # logs rather than artifacts -- yet a hit still skips the whole suite, # which is the single most expensive task in the graph. - name: Restore turbo cache if: ${{ needs.changes.outputs.frontend == 'true' }} - uses: actions/cache@v4 + uses: actions/cache@v6 with: path: .turbo/cache - key: turbo-test-${{ runner.os }}-${{ github.sha }} + key: turbo-test-${{ runner.os }}-${{ runner.arch }}-${{ steps.runtime.outputs.node }}-${{ github.sha }} restore-keys: | - turbo-test-${{ runner.os }}- + turbo-test-${{ runner.os }}-${{ runner.arch }}-${{ steps.runtime.outputs.node }}- - name: Test if: ${{ needs.changes.outputs.frontend == 'true' }} # Same filter rationale as frontend-build. Type errors are not this - # job's responsibility — the `typecheck` task in frontend-build owns - # them, which is why `test` no longer depends on `^typecheck`. + # job's responsibility -- frontend-build owns the `typecheck` task. + # `test` reaches dependency sources through the hash-only + # `^cache-inputs` edge (see turbo.json), so no `tsc` runs here. run: pnpm exec turbo test --filter='!@multica/docs' --filter='!@multica/mobile' # Aggregate gate. `frontend` is the status-check name the repository's @@ -204,7 +220,10 @@ jobs: # trivially in that case because every step is gated off. frontend: needs: [frontend-build, frontend-test] - if: ${{ always() }} + # `!cancelled()` rather than `always()`: a run superseded by a newer push + # is cancelled by the concurrency group above, and there is no reason to + # spend a runner reporting a verdict nobody will read. + if: ${{ !cancelled() }} runs-on: ubuntu-latest steps: - name: Check frontend job results diff --git a/turbo.json b/turbo.json index 3de9ad7e92..c1852127f4 100644 --- a/turbo.json +++ b/turbo.json @@ -43,16 +43,24 @@ "typecheck": { "dependsOn": ["^typecheck"] }, - // `^typecheck` is load-bearing here, but not for ordering: a turbo task - // hash only covers a workspace dependency's sources if a task edge - // reaches them. Without this edge, editing packages/views left + // Hash-only transit task. No package implements a `cache-inputs` script, + // so every node resolves to and nothing is ever executed -- + // but the edges still pull each dependency's file hashes into whatever + // depends on it. `dependsOn: ["^cache-inputs"]` makes that recursive, so + // the chain stays complete if a package ever gains a purely transitive + // dependency. This exists because a turbo task hash covers a workspace + // dependency's sources only when a task edge reaches them. + "cache-inputs": { + "dependsOn": ["^cache-inputs"] + }, + // Without a dependency edge, editing packages/views or packages/ui left // `@multica/web#test` and `@multica/desktop#test` byte-identical in hash, - // so a cached pass would be replayed for code that changed underneath. - // `typecheck` is the one task every package defines, which is what makes - // it the usable edge -- `^test` would miss packages/ui, which has no test - // script. Harmless to omit while nothing was cached; not once it is. + // so a cached pass would be replayed over changed code. `^cache-inputs` + // closes that without side effects: `^typecheck` would also work but drags + // three real `tsc --noEmit` runs (~223 CPU-seconds) into the test job, and + // `^test` would serialise the suites behind each other. "test": { - "dependsOn": ["^typecheck"] + "dependsOn": ["^cache-inputs"] }, // `lint` keeps no dependency edge: eslint here is not type-aware (no // `projectService` / `parserOptions.project` anywhere in