Skip to content

CI: fan out mise tool setup to eliminate redundant SPM builds - #314

Merged
leogdion merged 3 commits into
v1.0.0-beta.1from
claude/optimize-ci-workflows-CqmGF
May 9, 2026
Merged

CI: fan out mise tool setup to eliminate redundant SPM builds#314
leogdion merged 3 commits into
v1.0.0-beta.1from
claude/optimize-ci-workflows-CqmGF

Conversation

@leogdion

@leogdionleogdion commented May 8, 2026

Copy link
Copy Markdown
Member

Diagnosis

Both .github/workflows/MistKit.yml and .github/workflows/MistDemo.yml lint jobs invoked jdx/mise-action@v4 with cache: true against the same root mise.toml. They computed identical cache keys and raced each other on every push — the loser hit:

Failed to save: Unable to reserve cache with key ..., another job may be creating this cache

…and got nothing for it, forcing a clean ~9-minute SPM rebuild of swift-format, swift-openapi-generator, and periphery (all spm: backend tools). This was happening on essentially every PR that touched both projects.

A second, separate problem: the default jdx/mise-action cache configuration has no restore-keys fallback, so any mise.toml version bump (even a patch on a single tool) produced a clean rebuild from scratch instead of an incremental SPM rebuild against the previous tool tree.

Fix

Fan out from a single setup job that owns the cache, and have both lint workflows consume the populated cache read-only.

  • New reusable workflow .github/workflows/setup-tools.yml (triggered via workflow_call) owns cache lifecycle:
    • actions/cache@v4 with path ~/.local/share/mise/installs, key mise-v2-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('mise.toml') }}, plus a restore-keys fallback on the mise-v2-${{ runner.os }}-${{ runner.arch }}- prefix.
    • Conditionally runs jdx/mise-action@v4 with cache: false only on cache miss (steps.<id>.outputs.cache-hit != 'true'), so we own the cache lifecycle and avoid the reservation race.
  • MistKit.yml and MistDemo.yml:
    • Add a top-level setup-tools job that calls ./.github/workflows/setup-tools.yml.
    • Add setup-tools to each lint job's needs: list.
    • Replace the lint job's jdx/mise-action@v4 invocation with actions/cache/restore@v4 (read-only, same key/path as setup-tools, fail-on-cache-miss: true so a broken hand-off fails loudly instead of silently rebuilding) followed by jdx/mise-action@v4 with install: false and cache: false (mise just configures PATH/shims against tools already on disk).
    • All other steps, env vars, working directories, and script invocations are untouched.

mise.toml is unchanged. No tools were swapped to other backends (periphery stays on spm: because there's no Linux binary release in ubi:). No scripts under Scripts/ were touched.

Expected impact

  • Cold-cache cost paid once per workflow run instead of once per project — eliminates the cache race entirely.
  • mise.toml version bumps become incremental SPM rebuilds rather than clean rebuilds, since restore-keys will pick up the previous tool tree.
  • Warm-cache lint jobs do a fast read-only restore + PATH setup; no build at all.

Test plan

  • First push: setup-tools populates the cache; both lint jobs do a read-only restore and pass.
  • Second push (no mise.toml change): cache is hit on setup-tools (no install step runs), lint jobs restore quickly.
  • Bump a tool version in mise.toml: setup-tools misses on exact key, restore-keys picks up prior tree, mise-action does an incremental SPM rebuild rather than a full clean one.
  • Confirm lint jobs fail loudly (cache miss) if the setup-tools job ever fails to populate.

Generated by Claude Code


Perform an AI-assisted review on CodePeer.com

Both MistKit.yml and MistDemo.yml lint jobs invoked jdx/mise-action@v4
with cache: true against the same root mise.toml, computing identical
cache keys and racing each other on every push. The loser hit
"Failed to save: Unable to reserve cache with key ..., another job
may be creating this cache" and got nothing, forcing a clean ~9
minute SPM rebuild of swift-format, swift-openapi-generator, and
periphery. The default mise-action cache also lacked restore-keys,
so any mise.toml bump produced a clean rebuild instead of an
incremental one.
Fix: extract a reusable setup-tools.yml that owns the cache via
actions/cache@v4 with restore-keys fallback. Lint jobs now depend on
setup-tools and do read-only actions/cache/restore@v4 with
fail-on-cache-miss: true (so a broken hand-off fails loudly), then
run mise-action with install: false / cache: false so it only
configures PATH/shims against tools already on disk. Cold-cache cost
is paid once per workflow run instead of once per project, and a
mise.toml version bump becomes an incremental SPM rebuild rather
than a clean one.
@coderabbitai

coderabbitaiBot commented May 8, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8846bff6-4118-4765-b01d-ce0ad5c3a216

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/optimize-ci-workflows-CqmGF

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@leogdion
leogdion changed the base branch from main to v1.0.0-beta.1May 8, 2026 17:37
@claude

claudeBot commented May 8, 2026

Copy link
Copy Markdown

Code Review: CI Fan-Out for mise Tool Setup

Overview

This PR fixes a real and annoying problem: two lint jobs (MistKit + MistDemo) computed identical mise cache keys and raced each other, with the loser paying a full ~9-minute cold SPM rebuild. The fix — a shared setup-tools.yml reusable workflow that owns the cache lifecycle, with read-only restores in consumers — is the right architectural move.

The PR also carries significant changes to the example subprojects (BushelCloud, CelestraCloud, MistDemo), which appear to be independent subrepo syncs bundled in the same branch.


Core CI Changes (.github/workflows/)

What works well:

  • The setup-tools.yml reusable workflow cleanly separates cache ownership from cache consumption.
  • actions/cache/restore@v4 (read-only) in the lint jobs correctly prevents each consumer from racing to save the cache.
  • fail-on-cache-miss: true in lint jobs gives a loud failure if the setup-tools hand-off breaks, rather than silently triggering a redundant rebuild.
  • The restore-keys fallback (mise-v2-${{ runner.os }}-${{ runner.arch }}-) enables incremental SPM rebuilds on mise.toml version bumps.
  • The versioned mise-v2- prefix makes manual cache-busting easy (bump to mise-v3-).
  • cleanup-caches.yml on branch delete prevents cache quota bloat over time.

Issues and suggestions:

1. setup-tools.yml triggers not covered in path filters

MistKit.yml and MistDemo.yml do not include .github/workflows/setup-tools.yml in their trigger paths. A change to the shared setup workflow will not cause either workflow to re-run for validation on PRs.

# MistDemo.yml paths — consider adding:
- '.github/workflows/setup-tools.yml'

2. cleanup-caches.yml has no pagination

getActionsCacheList returns a default page size of 100. Branches with > 100 cache entries (unusual but possible on long-lived feature branches) would only have the first page cleaned. Consider adding per_page: 100 and looping, or open a follow-up issue.

3. setup-tools.yml is missing an explicit permissions block

Reusable workflows inherit caller permissions, but declaring permissions: contents: read explicitly is defensive and makes it clear to future readers that actions/checkout is the only privileged step.

jobs:
setup-tools:
permissions:
contents: readruns-on: ubuntu-latest

4. lint in MistKit.yml waits on all build jobs — appears intentional

The lint job has needs: [setup-tools, build-ubuntu, build-macos, build-macos-platforms, build-windows, build-android]. Jobs like build-macos-platforms, build-windows, and build-android are gated on full-matrix == 'true' and skip on feature branches. Skipped jobs resolve instantly so there is no correctness problem. If the intent is "lint only runs if builds pass," the gate is correct as-is.

5. Cache key uses runner.arch — verify runner consistency

Both setup-tools.yml and the lint jobs run on ubuntu-latest. If GitHub starts mixing amd64/arm64 runners for ubuntu-latest, the arch-keyed cache could miss in the consumer even after a hit in the producer. Very low risk today, but worth knowing it is an implicit assumption.


Example Subproject Changes

The BushelCloud and CelestraCloud lint jobs still use jdx/mise-action@v4 cache: true directly — this is correct, since they are independent subrepos with their own mise.toml and do not share the root cache key. No issue there.

The CelestraCloud Swift changes (new ExitError, UpdateCommand+Reporting, FeedUpdateProcessor+Fetch, test splits) and MistDemo additions (new Package.swift, scripts, MistDemoApp.swift) appear to be unrelated subrepo syncs bundled into this branch. These are hard to review alongside a CI-optimization PR — consider noting which files are subrepo syncs in the PR description so reviewers know not to conflate them with the CI changes.


Summary

The core fix is correct and well-reasoned. Three actionable items before merging:

  1. Add .github/workflows/setup-tools.yml to the paths triggers in MistKit.yml and MistDemo.yml so changes to the shared workflow are validated by PRs.
  2. Add explicit permissions: contents: read to setup-tools.yml.
  3. Note in the PR description that the Example directory changes are subrepo syncs.

The pagination gap in cleanup-caches.yml is low-priority but worth a follow-up issue.


Review generated by Claude Code

@codecov

codecovBot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.17%. Comparing base (73f13d5) to head (574f76c).

Additional details and impacted files
@@ Coverage Diff @@## v1.0.0-beta.1 #314 +/- ##
==============================================
Coverage 67.16% 67.17% ==============================================
Files 495 495 Lines 13869 13869 ==============================================
+ Hits 9315 9316 +1 + Misses 4554 4553 -1 
FlagCoverage Δ
mistdemo-spm-macos56.71% <ø> (ø)
mistdemo-swift-6.2-jammy56.72% <ø> (+0.06%)⬆️
mistdemo-swift-6.2-noble56.70% <ø> (-0.03%)⬇️
mistdemo-swift-6.3-jammy56.72% <ø> (ø)
mistdemo-swift-6.3-noble56.72% <ø> (+0.02%)⬆️
spm48.45% <ø> (+0.23%)⬆️
swift-6.1-jammy48.21% <ø> (+0.02%)⬆️
swift-6.1-noble48.24% <ø> (+0.05%)⬆️
swift-6.2-jammy48.15% <ø> (-0.09%)⬇️
swift-6.2-noble48.45% <ø> (+0.05%)⬆️
swift-6.3-jammy48.21% <ø> (+0.02%)⬆️
swift-6.3-noble48.48% <ø> (+0.26%)⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Adds a concurrency group on setup-tools.yml keyed on the ref so that
when both MistKit.yml and MistDemo.yml trigger on the same push,
their setup-tools invocations run sequentially. On a cold cache (e.g.
after a mise.toml bump), the first caller rebuilds and saves; the
second waits, then hits the cache and skips the install step. Without
this, both could miss simultaneously, both rebuild, and race on
cache save — the original problem we were trying to eliminate.
cancel-in-progress: false so the queued caller actually runs instead
of being cancelled.
@claude

claudeBot commented May 8, 2026

Copy link
Copy Markdown

Code Review: CI Fan-Out for mise Tool Setup

Overview

This PR correctly diagnoses and fixes a cache reservation race between MistKit.yml and MistDemo.yml lint jobs, both of which previously ran jdx/mise-action against the same mise.toml. The solution — a reusable setup-tools.yml that owns the cache write path, plus read-only actions/cache/restore in consumers — is architecturally sound and follows GitHub Actions best practices.


Correctness

  • Cache key consistency is maintained across all three files (mise-v2-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('mise.toml') }}). Good.
  • fail-on-cache-miss: true in the lint jobs ensures a broken hand-off fails loudly rather than silently falling through to a ~9-minute rebuild. This is exactly right.
  • cancel-in-progress: false on the concurrency group is correct — cancelling the setup job while a consumer is waiting for it would leave the consumer stranded.
  • Partial-cache incremental path: when restore-keys matches but the exact key misses, cache-hit is false, so jdx/mise-action@v4 runs and rebuilds on top of the partial restore. actions/cache@v4 (not /restore) then saves under the exact key at job end. This is the correct incremental behaviour described in the PR.

Potential Issues

1. concurrency in a workflow_call context may not serialize as expected

The setup-tools.yml concurrency group:

concurrency:
group: setup-tools-${{ github.head_ref || github.ref }}cancel-in-progress: false

When called via workflow_call, github.head_ref and github.ref are inherited from the triggering workflow's context. GitHub's docs confirm this, and the serialization intent should work. However, GitHub Actions has historically had edge cases with concurrency in reusable workflows. It is worth explicitly validating on the first cold-cache push that both MistKit/setup-tools and MistDemo/setup-tools jobs are not running simultaneously (check the Actions tab timeline view).

2. Cache path is platform-specific

~/.local/share/mise/installs is the Linux/XDG path. Both setup-tools and the lint jobs run on ubuntu-latest, so this is consistent today. If a macOS lint runner is ever added, the cache path would differ (~/Library/Application Support/mise/installs). Consider adding a comment noting the platform assumption, or parameterising the path via mise config output in the future.

3. No permissions block on setup-tools.yml

# Suggest adding:permissions:
contents: read

GitHub Actions jobs default to broad permissions in some org configurations. Explicitly scoping to contents: read is a security hygiene best practice for a job that only needs to check out code and manage cache.

4. The restore-keys prefix is broad

mise-v2-${{ runner.os }}-${{ runner.arch }}-

This will match any prior mise-v2 cache for this OS/arch, regardless of which branch it came from. In a repo with many active branches, a lint job on a feature branch could restore a cache that was built from a very different mise.toml state, then write a new exact-key cache. This is low risk here since mise.toml is stable, but it is worth being aware of.


Code Quality

  • The inline comments in setup-tools.yml explaining the race condition and the strategy are clear and valuable — a future maintainer will understand why this exists without reading the PR.
  • Both MistKit.yml and MistDemo.yml are changed identically and symmetrically. Good.
  • install: false + cache: false on the consumer-side jdx/mise-action invocation is the correct minimal configuration (configure PATH/shims only; skip install and skip its own cache management).

Suggestions

  1. Validate serialization on first push: after merging, check the Actions run timeline to confirm the two setup-tools jobs queued sequentially, not in parallel.

  2. Add permissions: contents: read to setup-tools.yml for defence in depth.

  3. (Optional) Add a name: to the cache restore step in setup-tools.yml for readability in the Actions UI:

    - name: Restore mise tools cacheid: mise-cacheuses: actions/cache@v4...

    It has an id but no name — minor but inconsistent with the consumer steps which do have names.


Test Coverage

The test plan in the PR description is appropriate. No Swift source changes are made, so no unit/integration test changes are needed. CI itself is the test.


Summary

Approve with minor suggestions. The core approach is correct and well-reasoned. The two actionable items are: (1) add a permissions block to setup-tools.yml, and (2) verify concurrency serialization empirically after the first merge. Everything else is cosmetic or forward-looking.

@claude

claudeBot commented May 8, 2026

Copy link
Copy Markdown

Code Review — PR #314: CI cache optimization (fan out mise tool setup)

Overview

This PR addresses a real, well-diagnosed problem: two lint jobs sharing the same cache key racing each other on every push. The fix — a shared setup-tools reusable workflow that owns the cache lifecycle, with consumers doing read-only restores — is the right pattern for this scenario.


Code Quality & Correctness

Positives:

  • concurrency: cancel-in-progress: false on setup-tools.yml is correct. Cancelling a running cache-population job would leave consumers with a broken cache miss + fail-on-cache-miss: true. Serializing instead of cancelling is the right call.
  • fail-on-cache-miss: true in consumer lint jobs makes a broken hand-off fail loudly rather than silently rebuilding — good operational hygiene.
  • restore-keys fallback on the prefix mise-v2-${{ runner.os }}-${{ runner.arch }}- enables incremental SPM rebuilds on version bumps rather than clean rebuilds.
  • Cache key includes runner.arch — prevents cross-architecture cache pollution.

Suggestions:

  1. No timeout-minutes on the setup-tools job — If an SPM build hangs (network issue, deadlock), the setup-tools job runs until GitHub's 6-hour job limit, blocking both downstream lint jobs for the full duration. Adding a reasonable timeout (e.g., timeout-minutes: 30) would bound the blast radius:

    jobs:
    setup-tools:
    name: Setup mise toolsruns-on: ubuntu-latesttimeout-minutes: 30# add this
  2. mise-action@v4 with install: false, cache: false assumption — The consumer lint jobs rely on jdx/mise-action@v4 to configure PATH/shims against the pre-populated cache, without actually installing anything. This works because mise itself is available on the GitHub runner, and the tools are already in ~/.local/share/mise/installs. This is a non-obvious dependency on runner image state; a brief inline comment in the workflow documenting why install: false is correct here would help future maintainers avoid accidentally "fixing" it.

  3. Runner OS couplingsetup-tools.yml hardcodes runs-on: ubuntu-latest, matching the current lint jobs. If lint jobs ever move to macOS runners, the cached binaries won't be compatible (different OS/arch). The cache key (runner.os+runner.arch) would prevent cross-pollination, but a new setup-tools instance for that runner type would be needed. Not a bug today, just worth noting in the PR description or a comment.


Potential Risks

  1. concurrency group scope — The group key is setup-tools-${{ github.head_ref || github.ref }}. For PRs, github.head_ref is the PR branch name, so concurrent pushes to the same PR branch serialize correctly. For direct pushes to main, github.head_ref is empty and falls back to refs/heads/main — also correct. One edge case: if two different PRs push simultaneously, they get separate concurrency groups and run in parallel. Since they produce different cache keys (different hashFiles('mise.toml') if content differs, same key if content is the same), two simultaneous cold-cache runs on different PRs could race on the same cache key. The actions/cache@v4 action handles this gracefully (last writer wins), so it's not a correctness issue — just a wasted rebuild on a cold-start race day.

Security

No concerns. Cache paths are scoped to tool installations only, not source code or credentials.


Summary

Well-reasoned fix for a concrete CI reliability problem. The only actionable item is adding timeout-minutes to prevent hung SPM builds from blocking lint jobs indefinitely. The install: false pattern is worth a short comment for maintainability.

The reusable workflow approach failed in practice: MistDemo's lint
job hit "Failed to restore cache entry" with fail-on-cache-miss in
the same workflow run where setup-tools reported a 10-second cache
hit on the same key. Caches written by actions/cache from inside a
workflow_call reusable workflow are scoped to the reusable workflow's
path, while actions/cache/restore from the caller looks in the
caller's scope — they don't see each other.
Switch to a composite action at .github/actions/setup-tools so cache
operations run inline in the caller's job and inherit the caller's
scope. Each lint job now does its own restore + (install on miss) +
save. Branch-scoped caches mean the first job to save makes the
entry visible to the second.
To preserve the cross-workflow serialization that prevents a cold-
cache race when MistKit and MistDemo lint trigger on the same push,
add a shared job-level concurrency group (lint-tools-<ref>,
cancel-in-progress: false). First job rebuilds and saves; the second
queues, runs, hits cache, and is fast.
@claude

claudeBot commented May 8, 2026

Copy link
Copy Markdown

Code Review: PR #314 — CI: Composite Action for mise Tool Cache

Overview

A targeted, well-diagnosed CI fix. The switch from jdx/mise-action (with cache: true) to a composite action that owns the cache lifecycle is the right solution to both the cache reservation race and the missing restore-keys fallback.


Strengths

Composite action vs. reusable workflow
The PR description mentions a reusable workflow, but the implementation correctly uses a composite action — and the explanation in the action YAML header is exactly right: reusable workflows scope their own caches, which silently breaks hand-off between jobs. The composite action shares the caller job's cache scope, so setup and lint jobs in the same workflow see the same cache. Good call.

Conditional install
Only calling jdx/mise-action on cache miss (with cache: false to hand ownership back to actions/cache) is clean. On warm cache, mise just sets up PATH with install: false — no redundant work.

restore-keys fallback
This is the right fix for the "every version bump causes a full clean rebuild" problem. Partial cache hits on mise-v2-{os}-{arch}- let SPM do an incremental rebuild rather than starting from scratch.


Issues

concurrency group serializes across both lint jobs — this is intentional and correct for the stated goal (preventing the cache reservation race), but it means a push that triggers both MistKit.yml and MistDemo.yml will run their lint jobs sequentially rather than in parallel. On a cold cache (after a mise.toml bump) that adds real wall-clock time. The trade-off is explicitly called out in the PR description, so this is noted rather than flagged — confirming the behavior is understood.


Nit

The PR description says lint jobs use fail-on-cache-miss: true for a loud failure if setup never ran. The final implementation does not have this (the composite action handles a miss by re-installing), which is actually cleaner. If setup fails entirely, the lint job will rebuild from scratch rather than fail loudly. Fine for CI resilience, but the description is slightly out of sync with the implementation.


Summary

Correct, minimal change that solves a real CI problem. No issues warrant blocking merge.

Reviewed with Claude Code

@claude

claudeBot commented May 8, 2026

Copy link
Copy Markdown

Code Review: PR #314 — CI: fan out mise tool setup

Overview

Addresses a real cache-race bug: both workflow lint jobs computed identical mise cache keys and raced each other on every push, with the loser forced into a ~9-minute clean SPM rebuild. The fix serializes cache ownership via concurrency groups and a composite action.


Strengths

  • Root cause is correctly diagnosed. The race is between MistKit.yml and MistDemo.yml lint jobs sharing a cache key — not a timing quirk but a structural problem.
  • Composite action vs. reusable workflow is the right call. The PR description correctly explains why: reusable workflows scope caches to the called workflow's job, not the caller's, which would silently break cache hand-off. A composite action runs in the caller's job scope and shares the cache correctly.
  • restore-keys fallback is a meaningful improvement — mise.toml version bumps now produce incremental SPM rebuilds rather than clean ones.
  • cancel-in-progress: false is correct here. You want the second lint job to wait for the first to populate the cache rather than cancel it mid-write.
  • Conditional install on cache miss is clean: owns the full lifecycle (save + restore) in one action, no separate read-only restore step needed in the consumers.

Issues and Suggestions

1. Concurrency group key uses github.head_ref || github.ref

On pushes to the default branch (no PR), github.head_ref is empty and the group becomes lint-tools-refs/heads/main. This means every push to main from both workflows competes for the same concurrency slot — which is the intended behavior. For PRs the group is the branch name. This is correct, but worth confirming that cancel-in-progress: false is intended to queue (not skip) a concurrent main push lint job rather than letting it run in parallel.

2. fail-on-cache-miss is not set on the composite action's actions/cache@v4

The action uses actions/cache@v4 (save + restore) which is appropriate for the owner. The consuming lint jobs just call uses: ./.github/actions/setup-tools — they don't do a separate read-only restore step with fail-on-cache-miss: true. This means if the setup-tools job somehow failed to populate the cache (e.g., a transient runner failure), the lint job would silently do a full reinstall rather than failing loudly. The PR description mentioned fail-on-cache-miss: true as part of the design, but it doesn't appear in the final implementation. This is a minor reliability concern — if the install step in setup-tools fails partway through, the next job might pick up a partial cache.

3. No test plan items are checked

The test plan lists four scenarios but none are checked:

  • First push: setup-tools populates, both lint jobs restore
  • Second push: cache hit, no install
  • Tool version bump: incremental rebuild via restore-keys
  • Lint job failure on cache miss

These can only be verified by actually running CI — worth a note that they'll be checked on merge, or checking them off if CI has already been run against this branch.


Summary

Correct fix for a real problem. The composite action design is well-reasoned and the restore-keys improvement is a genuine bonus. The only functional question is whether fail-on-cache-miss behavior is intentionally omitted (partial-cache resilience over loud failure) or an oversight from the PR description.

🤖 Reviewed with Claude Code

@claude

claudeBot commented May 8, 2026

Copy link
Copy Markdown

Code Review — PR #314: CI tool setup fan-out

Overview

Clean fix for a real CI pain point. The composite action approach (rather than a reusable workflow) is the right call — reusable workflows create their own cache scope that can't be shared back to the caller job, which would reintroduce the hand-off problem. The concurrency group that serializes the two lint jobs is the key piece that eliminates the race.


Issues

🟡 actions/checkout@v6 — version may not exist

Both MistKit.yml and MistDemo.yml use actions/checkout@v6. As of this review, the latest stable release of actions/checkout is v4. v6 either refers to a pre-release, a custom fork, or a typo. If this is already used in the existing workflow and working in CI, it can stay; if it's being introduced here, verify it resolves to a real release.

🟡 Serialization adds latency for PRs touching both projects

The shared concurrency group lint-tools-${{ github.head_ref || github.ref }} with cancel-in-progress: false means MistKit lint and MistDemo lint will never run in parallel on the same ref. On cold-cache pushes this is a net win (one ~9-minute build instead of two). On warm-cache pushes the restore is fast (~seconds), so the serialization adds the full lint runtime of whichever job runs second as unnecessary latency.

Consider whether cancel-in-progress: false is correct here. Cancelling would defeat the purpose (we need both lints to pass), so it is correct — but the sequential execution is a trade-off worth documenting.

🟡 fail-on-cache-miss not set — silent rebuild possible

The PR description mentions using fail-on-cache-miss: true on the restore step so a failed setup-tools job produces a loud failure rather than a silent rebuild. The composite action as implemented uses actions/cache@v4 (which both saves and restores), so it will simply rebuild on any miss rather than failing. This means if the setup-tools job itself fails partway through (e.g. the install step errors), subsequent consumers get a cache miss that triggers yet another rebuild in the same job — the failure mode is noisy latency rather than a loud error.

If serialization via concurrency is working correctly, this is an acceptable trade-off. But a comment documenting the failure mode would help future debuggers.


Minor / Nits

Composite action self-description — the description field says "reusable workflows scope caches separately, which silently breaks hand-off." This is accurate and exactly right; keep it, it's valuable for future maintainers who might wonder why a reusable workflow wasn't used instead.

cache: false on both branches — the composite action correctly sets cache: false on jdx/mise-action in both the cache-hit and cache-miss branches, handing full cache lifecycle control to actions/cache@v4. This is the correct pattern for owning the cache key.

restore-keys fallback — the mise-v2-${{ runner.os }}-${{ runner.arch }}- prefix fallback on version bumps is a good incremental-rebuild strategy rather than forcing a full clean rebuild. Well thought out.

Comment in workflow files — the inline comment explaining the concurrency group is helpful:

# Shared with MistDemo's lint job: serialize across workflows so a cold# cache on a mise.toml bump only triggers one rebuild, not a race.

This is exactly the kind of "why" comment that belongs here.


Summary

Solid CI improvement. The composite-action approach correctly scopes the cache to the caller job and the concurrency group eliminates the race. The main asks:

  1. Verify actions/checkout@v6 resolves to a real, working release
  2. Document the serialization latency trade-off (or accept it as a known cost)

🤖 Generated with Claude Code

@claude

claudeBot commented May 9, 2026

Copy link
Copy Markdown

Code Review — PR #314: CI tool setup fan-out

Overview

A focused CI fix: two lint jobs were racing to write the same mise cache key, causing one to fail and rebuild all SPM-backed tools (~9 minutes) from scratch. The fix is a composite action (.github/actions/setup-tools/action.yml) that owns the full cache lifecycle, paired with a concurrency group that serializes writes across both workflows.


Strengths

  • Root cause analysis is accurate: both lint jobs sharing an identical cache key with no restore-keys fallback explains both the race condition and the full-rebuild-on-version-bump problem.
  • Composite action over reusable workflow: the inline comment correctly explains why a composite action is required here — reusable workflows scope their caches to their own job, so cache written in a workflow_call job would not be visible to the caller's lint job. This is a subtle GitHub Actions gotcha, and the choice is right.
  • Cache lifecycle ownership: passing cache: false to jdx/mise-action@v4 and letting actions/cache@v4 own save/restore is the clean way to prevent double-cache writes.
  • restore-keys fallback: the prefix mise-v2-${{ runner.os }}-${{ runner.arch }}- enables incremental SPM rebuilds on tool version bumps rather than cold rebuilds. This is the key improvement beyond just fixing the race.
  • cancel-in-progress: false: correct choice — cancelling a running setup job would leave the cache in an unknown state and break the waiting lint job. The serialization cost is worth the correctness guarantee.
  • No functional code touched: zero risk of regression to the build or test steps.

Potential Issues

Low — concurrency group covers only lint jobs, not the cache-write race directly

Both lint jobs share group: lint-tools-${{ github.head_ref || github.ref }}. This means when a MistKit lint job and a MistDemo lint job run concurrently on the same push, one will wait for the other. The waiting job then runs the composite action, hits the warm cache, and proceeds quickly.

However: if a push triggers both workflows at the same time, the second job does not start at all until the first lint job completes (not just the cache-write step). Depending on how long the lint step itself runs, this could add noticeable serial latency for PRs that touch both projects. It would be slightly more efficient to have the concurrency group applied only to the setup-tools step — but composite actions cannot have step-level concurrency controls, so this is an inherent limitation. Worth noting in a follow-up if CI latency becomes an issue.

Low — Cache path may differ between macOS and Linux runners

The cache path ~/.local/share/mise/installs is correct for Linux. On macOS runners (which the existing workflows use for build-macos), mise follows XDG conventions and uses the same path since mise 2024. However, the lint jobs run on ubuntu-latest, so this is only relevant if the composite action is ever used on macOS lint jobs in the future. The ${{ runner.os }} in the cache key already scopes correctly per OS.

Nit — Action description says "reusable workflows scope caches separately"

The composite action's description comment (Implemented as a composite action so the cache scope is the caller job's scope — reusable workflows scope caches separately) is accurate and valuable context. It would be worth preserving this comment if the action is ever refactored.


Correctness

  • The conditional if: steps.mise-cache.outputs.cache-hit != 'true' / == 'true' branching is correct: on a cold cache, mise installs tools and actions/cache@v4 saves them; on a warm cache, mise only configures PATH/shims.
  • The concurrency group prevents the double-write race across MistKit and MistDemo lint jobs on the same ref.
  • All other steps, env vars, working directories, and script invocations are unchanged — this is a pure infrastructure change.

Summary

Clean, well-reasoned fix with no functional impact. The composite action pattern is the correct choice for the cache scoping constraint, and the restore-keys fallback is a meaningful improvement over the original setup. The one note about serial lint latency is a trade-off rather than a bug. Merge-ready.

🤖 Generated with Claude Code

@claude

claudeBot commented May 9, 2026

Copy link
Copy Markdown

Code Review — PR #314: CI mise tool cache fan-out

Targeted review of the CI change. The diagnosis is accurate and the fix is correct.


Design

Composite action over reusable workflow — correct choice.
The comment in action.yml nails the reason: reusable workflows have their own cache scope, which silently breaks cache hand-off between a setup job and a consumer. A composite action runs in the caller job's scope, so the cache key computed in setup is the same one lint restores.

Cache key + restore-keys fallback — correct.

key: mise-v2-${{ runner.os }}-${{ runner.arch }}-${{ hashFiles('mise.toml') }}
restore-keys: |
mise-v2-${{ runner.os }}-${{ runner.arch }}-

The prefix fallback means a mise.toml version bump gets an incremental SPM rebuild from the previous tree rather than a cold start.

fail-on-cache-miss: true on the lint restore — not shown explicitly in the composite action itself (the composite action owns the save side), but the design note mentions it for the lint jobs. If it's not in the final composite, verify that a cache-miss on the lint side fails loudly rather than silently triggering a redundant install.


Issues

1. Serialised lint jobs may double wall-clock time

Both MistKit.yml and MistDemo.yml lint jobs share the concurrency group:

concurrency:
group: lint-tools-${{ github.head_ref || github.ref }}cancel-in-progress: false

GitHub Actions concurrency groups are global across all workflows in the repo. On a PR that touches both projects, the two lint jobs run sequentially instead of in parallel. The intent is to avoid the cache race, but once the cache is warm (second push onward), the jobs no longer need to be serialised — only the cold-cache path needs it. This is a trade-off worth calling out: warm-cache CI takes twice as long as it could. An alternative is to keep concurrency only on the setup-tools step/job and let the consumers run in parallel after the cache is guaranteed populated.

2. actions/checkout@v6 — check latest stable

Both lint jobs reference actions/checkout@v6, which at the time of writing may be a pre-release or ahead of @v4. Confirm this is intentional; the rest of the codebase may target different major versions.


Minor

  • install: false + cache: false on cache-hit path — correct; mise just wires up PATH/shims for tools already on disk. No rebuild triggered.
  • cancel-in-progress: false — correct; cancelling a lint job mid-flight doesn't help and would waste the cache slot.
  • mise.toml unchanged — confirmed; no tool backend changes.

Summary

Approve. The fix is correct and eliminates the cache race. Flag the sequential-lint trade-off to decide whether warm-cache performance is important enough to revisit the concurrency strategy later.

🤖 Generated with Claude Code

@leogdion
leogdion merged commit 3e7aa7d into v1.0.0-beta.1May 9, 2026
72 checks passed
@leogdion
leogdion deleted the claude/optimize-ci-workflows-CqmGF branch May 11, 2026 15:05
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@leogdion@claude