Repository navigation
ci(test): guard the mise cache settings and save the rocks cache only on push - #579
Conversation
…only on push A spec reads .github/workflows/test.yml and checks the cache settings of the Set up mise step: cache_key hashes only mise.lock and the step has no restore-keys, cache_save is true only on a push, and MISE_HTTP_RETRIES is an integer of at least 6. actionlint and zizmor accept a workflow that loses any of them. actions/cache saves the Lua rocks cache in its post step on every event, including pull_request and merge_group, after the PR's own scripts ran. The step is now actions/cache/restore plus actions/cache/save at the same pinned SHA. The save step runs only on a push, and only after a restore miss. The same spec checks that split. _Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
|
@codex review |
|
@codex security review |
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…ks cache step
The spec found a setting by its key anywhere in the step, so cache_key or
cache_save moved under env:, or MISE_HTTP_RETRIES moved under with:, kept it
green. It now looks for each setting in its with: or env: block and requires
hashFiles('mise.lock') inside ${{ }}.
The rock cache rules now name the tree in any spelling (./.luarocks, an
absolute path, a line of a multi-line path): a second actions/cache step for
it fails, and so does a restore-keys on the restore step. Both paths must be
.luarocks. The comment above the restore step says that actions/cache saves
only after a miss.
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…kflow spec
A double-quoted `uses: "actions/cache@..."` hid a second step that caches
.luarocks from the rock-cache rule, so the spec now strips a leading quote
before it matches the action name.
The mise cache_key must now match mise-v<n>-{{platform}}-{{version}}-
${{ hashFiles('mise.lock') }} as a whole. A key that drops {{version}} or adds
text that changes on each run (${{ github.run_id }}) fails. The comment of
caches_rock_tree now says what the function checks.
_Generated with xum • Model: anthropic:claude-sonnet-5-5 • Thinking:
|
@codex review |
|
@codex security review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
🛡️ Codex Security ReviewSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
…tep it cannot read (coder#591) ## Why coder#579 added a spec that reads `.github/workflows/test.yml` as text and checks the cache settings. The independent reviews of its heads found gaps in how the spec reads the file. Each gap lets a risky edit pass or fails a harmless one. 1. A step written with a quoted key (`"uses": actions/cache@...`) was not seen, so a second step that caches `.luarocks` passed. 2. A second `jdx/mise-action` step with the default cache settings would save the tool cache on a pull request. Nothing failed. 3. The rock restore key was only checked to contain `hashFiles('mise.toml')`. Text that changes on each run (`-${{ github.sha }}`) kept the spec green and the cache would never hit. 4. The spec failed edits that YAML reads the same: a quoted `cache_key`, a quoted restore path, a quoted `uses:`. 5. A step in flow style (`- { uses: actions/cache@..., with: { path: .luarocks, key: ... } }`) was not seen. ## What changed Only `tests/unit/workflow_cache_spec.lua` (+98, -19). The workflows are unchanged. The spec still has no YAML parser. 1. `field()` accepts a quote around the key (`"uses":` or `'uses':`). 2. A new test: every `jdx/mise-action` step turns its cache off (`cache: false` or `cache_save: false`) or saves only on a push. 3. The rock restore key must be `luarocks-${{ runner.os }}-${{ hashFiles('mise.toml') }}` as a whole. A version after the prefix (`luarocks-v2-`) is allowed, as `v1` is in the mise key. 4. `field()` strips one pair of YAML quotes from a value, and reads `''` in a single-quoted value as `'`. 5. A new test fails closed. It counts the lines of the file that are not comment lines and name `actions/cache` or `jdx/mise-action` (a trailing comment stays in the line), and the steps that have the name as their `uses:`. The two counts must be equal. So the spec fails for a flow-style step or list of steps, even with a ` #` inside a quoted value, and for the name on any other line (a step `name:`, a `run:` line, a trailing comment). The line count ignores the letter case and the `uses:` match does not, so an upper-case spelling such as `Actions/cache@...` fails too. The one exception is a name written with a YAML escape (`"actions\x2fcache@..."`), which the spec does not decode (see Not covered). 6. The helper that tells whether a step uses an action requires `@` or `/` after the action name. A similarly prefixed repository such as `jdx/mise-action-fork@...` no longer counts as `jdx/mise-action`, so it cannot satisfy the line count or the check that a `jdx/mise-action` step exists. ## Evidence Before is the spec of coder#579 (10 tests, merged as `b450eb91`), after is this branch (12 tests). Each mutant is the workflow of coder#579 with one edit that applies exactly once. `m`, `L`, `X` and `Y` are the mutants of coder#579, `Z1` to `Z12` are the probes of its last review, `N1` to `N20` are mine (there is no `N6`), `P1` to `P26` are the probes of the review of this PR (the ones in the tables below), and `Q1` to `Q4` come from a Codex finding on the second head. | Item | Mutant | Before | After | | --- | --- | --- | --- | | 1 | `Z9` and `N5`: a second `actions/cache@` step for `.luarocks` with a quoted key `"uses":` or `'uses':` | green | red (6 rock-cache tests) | | 2 | `Y3`: a second `jdx/mise-action` step with the default cache settings. `N10`: it sets `cache_save: true`. `N11`: it sets `cache_save` to `!= 'pull_request'` | green | red (1 test) | | 2 | Controls `N7` (`cache: false`), `N8` (`cache_save: false`), `N9` (the push-only expression in quotes) | green | green | | 3 | `Y4`: `-${{ github.sha }}` on the rock key. `N14`: an extra segment in it | green | red (1 test) | | 3 | Controls `N12` (the key in quotes), `N13` (`luarocks-v2-`) | green | green | | 4 | `Z1` (quoted `cache_key`), `Y5` (quoted restore path), `Z7` (quoted `uses:` on the restore step), `N15` (quoted `cache_save`), `N16` (quoted save `if:`), `N17` (single-quoted mise key with `''`), `N19` (quoted save path), `N20` (quoted step name) | red | green | | 4 | Control `N18` (`MISE_HTTP_RETRIES: '6'`) | green | green | | 5 | `N1`: a flow-style second cache step for `.luarocks`. `N2`: the same for another path, which fails closed on purpose. `N3`: a flow-style second `jdx/mise-action` step. `Z6`: `Actions/cache@` | green | red (1 test) | | 5 | `P23` and `P25`: a flow-style second cache step for `.luarocks` with a ` #` inside a quoted value. `P24`: the same for a second `jdx/mise-action` step. `P26`: the same step as `P23` without the `#` | green | red (1 test) | | 6 | `Q1`: the setup step uses `jdx/mise-action-fork@...`. `Q2`: a step `actions/cache-extra@...` for another path. `Q3`: the same step for `.luarocks` | green | red (1 test, and 2 for `Q1`) | | 6 | Control `Q4`: the setup step uses `jdx/mise-action/sub@...` | green | green | | 5 | `N4`: a flow-style `with:` on the rock restore. It fails the path and key tests, as it did before, and not the new test | red | red | | 5 | Harmless edits that the spec fails on purpose: `P3` (a trailing comment that names `actions/cache` on a `run:` line), `P4` (the name in a step `name:`), `P5` (the name in a `run:` line), `P13` (`cache: False` on a second `jdx/mise-action` step) | green | red (1 test) | Counts over all 102 mutants (the 86 of the first head, `P1`, `P1b`, `P2`, `P3`, `P4`, `P5`, `P13`, `P14`, `P23` to `P26`, and `Q1` to `Q4`). Before, 59 are red and 43 are green. After, 73 are red and 29 are green. 51 stay red, 22 turn red, 8 turn green and 21 stay green. Of the 29 green mutants, 8 are harmless edits that used to fail, 13 are controls (`m12`, `X8`, `X19`, `X23`, `Z2`, `Z3`, `N7`, `N8`, `N9`, `N12`, `N13`, `N18`, `Q4`), and 8 are gaps (`X3`, `X10`, `Z10`, `Z11`, and the four YAML escapes `P1`, `P1b`, `P2`, `P14`, see below). Five of the 51 red mutants (`m2`, `m3`, `X1`, `X11`, `X14`) and the tree before coder#560 now also fail the new `jdx/mise-action` test next to the test that failed before. No other test result changed. | Claim | Evidence | | --- | --- | | The spec passes on main (`b450eb91`) with this change | 12 successes, 0 failures | | It still fails on the trees before coder#579 (`b43d1c23`, `c7e2eb4e`) | 6 successes, 6 failures: the 6 rock-cache tests | | The workflows are valid | The workflows are identical to main. `actionlint` 1.7.12 on all 5: exit 0. `zizmor` 1.25.2 `--offline` on all 5: no findings (25 suppressed). On the mutants, `actionlint` exits non-zero only on `L8`, `L9` and `X3`, and `zizmor` only on `L10` and `L11`, as for coder#579. `mise run format-check`: exit 0 (441 files, 0 changed) | | Gate | `mise run clean && mise run all` exits 0 in 540 s: 1904 successes, 0 failures, 0 errors, 35 pending (seed 1794532755). It ran on the merge of main (`692b460c`) into this branch. `luacheck`: 0 warnings, 0 errors in 198 files. The stub `claude` and GNU `timeout` first on `PATH` | ## Not covered - The spec does not see a rock path without the text `.luarocks` (a glob such as `.luaroc*`, `Z10`, or a parent folder, `Z11`). The save step is not checked for its place in the job, so moving it after the tests keeps the spec green (`X10`). A second `cache_save: true` key is caught by `actionlint` and not by the spec (`X3`). - The new tests also fail harmless edits, so a maintainer edits the spec with the workflow. Examples are a flow-style step for another path (`N2`), the name of an action in a step `name:` (`P4`), in a `run:` line (`P5`) or in a trailing comment (`P3`), and `cache: False` on a second `jdx/mise-action` step (`P13`). The action reads `False` as false and the spec compares the text `false`. A flow-style `with:` on a step the spec reads fails the path and key tests (`N4`), as it did before. - A name written with a YAML escape (`"actions\x2fcache@..."`, `"actions\u002fcache@..."`, `"jdx\x2fmise-action@..."`, or a path `"\x2eluarocks"`) is not decoded, so the spec does not see it (`P1`, `P1b`, `P2`, `P14`). The spec guards against accidental edits, and an escape spelling is not an accident, so it has no reader for it. A double-quoted `uses:` split by an escaped line break (`"actions/\` and then `cache@..."`) falls in the same class. - GitHub ignores the letter case in the owner and the repository of `uses:`, as the runner log in actions/runner#3731 shows. So `Actions/cache@...` is a real cache step, and the line-count test rejects it. The spec does not read it as a rock-cache step. - The spec reads the rock path as text, so a path that comes from a YAML alias (`with: *rocks` on a second `actions/cache` step, where the restore step's `with:` has the anchor `&rocks`) or from an expression such as `${{ format(...) }}` is not seen. GitHub supports anchors and aliases. An alias is a deliberate structure, like an escape, not an accidental edit, so the spec has no reader for it. - `field()` also accepts a quote on one side of a key (`cache_save": ...`, `cache': false`). YAML reads that as another key (`cache_save"`), so the action ignores it and uses its default (`true`). The spec reads it as `cache_save`, so a Set up mise step written this way passes this spec (the spec on main fails it), and so does a second `jdx/mise-action` step with `cache': false`. actionlint 1.7.12 exits 0 on both. A follow-up PR requires a matching pair of quotes. - The spec fails on a tree made with `git archive`, because `.gitattributes` export-ignores `.github`. The failure message says so. --- _Generated with [`xum`](https://github.com/coder/xum) • Model: `anthropic:claude-sonnet-5-5` • Thinking: `xhigh`_
Why
#560 keyed mise-action's tool cache on
mise.lock, saved it only on a push, and raised mise's download retries. Its independent review found two gaps, which this PR closes.actionlintandzizmorstayed green when a later edit removedcache_keyorMISE_HTTP_RETRIES, or loosenedcache_save. A removedcache_keyorMISE_HTTP_RETRIEScan show up later as a slow or failedSet up misestep. A loosenedcache_saveshows nowhere: no step gets slow or fails.actions/cachesaves in its post step onpull_requestandmerge_groupruns too, after the PR's own scripts ran. GitHub scopes cache entries by ref, so an entry saved by a PR run cannot reach main. No run has shown a bad entry. This change removes the write path by construction, as ci(test): key mise-action's tool cache on mise.lock and raise mise's HTTP retries #560 did for the tool cache.What changed
tests/unit/workflow_cache_spec.lua(new, 10 tests) reads.github/workflows/test.ymlas text. It uses plain Lua string matching, no YAML library and no network. Comment lines do not count, so a setting that sits only in a comment fails. A step that is renamed fails closed (no step is named ...). The spec finds each setting in its place: a key of the step at eight spaces of indent, and an entry of thewith:orenv:block at ten. A setting that moves to another block fails.Set up misestep:cache_keyis underwith:and matchesmise-v<n>-{{platform}}-{{version}}-${{ hashFiles('mise.lock') }}as a whole (<n>can change). So it hashes onlymise.lock(onehashFiles(, no{{file_hash}}, no{{default}}), and no text that changes between runs can be added to it. The step has norestore-keys.cache_saveis underwith:and is exactly${{ github.event_name == 'push' }}.MISE_HTTP_RETRIESis underenv:and is an integer of at least 6.actions/cache(plain,restoreorsave, with or without a quote around the name) and has a line that contains the text.luarocks(the path as.luarocks,./.luarocks,~/.luarocks, an absolute path, or one line of a multi-linepath) must be, in order, oneactions/cache/restoreand oneactions/cache/save. So a second step for the tree written with a plainuses:key, or the old singleactions/cache@step, fails. Both steps pin the same 40-character SHA and both use the path.luarocks. The restore key hashesmise.toml, and the restore step has norestore-keys. The save key is the restore step'scache-primary-keyoutput. The save step'sif:is exactlygithub.event_name == 'push' && steps.luarocks-cache.outputs.cache-hit != 'true'..github/workflows/test.yml: theCache Lua test rocksstep (actions/cache) becomesRestore Lua test rocks(actions/cache/restore,id: luarocks-cache, same key) beforemise run setup, andSave Lua test rocks(actions/cache/save) after it. Both use the SHA thatactions/cachehad (27d5ce7f..., v5.0.5). The save step runs only on a push to main and only when the restore did not find the exact key. The comment above the restore step says what the old step did: it saved after a miss, on every event.README.md and DEVELOPMENT.md do not describe the CI cache, so they are unchanged.
Evidence
All numbers are from runs on this head (
cc903344) unless a row says otherwise.37efd357), whose workflow is the same except for one comment523ee200)Set up misetests (key,cache_save, retries) and all 6 rock-cache testsc7e2eb4e, andb43d1c23,7dbad04band4005ab66give the same)Set up misetests pass, as they shouldactionlintandzizmorexit 0 on all 6m1tom12,L1toL12,X1toX23,Y1toY8): 47 red, 4 controls and 4 known gaps green.actionlintandzizmorexit 0 on 43 of the 47 red mutantsm12(retries"7"),X8(retries6without quotes),X19(anactions/cachestep for another path),X23(a secondcache_saveunderenv:next to the right one): 10 successes eachactionlint1.7.12 on all 5 workflows: exit 0 on the head and on main (b43d1c23andc7e2eb4e).zizmor1.25.2--offlineon all 5: no findings (25 suppressed), the same as main.mise run format-check: exit 0 (436 files, 0 changed)mise run clean && mise run allexits 0 in 1318 s: 1847 successes, 0 failures, 0 errors, 33 pending (seed 1794518856).luacheck: 0 warnings, 0 errors in 194 files. The stubclaudeand GNUtimeoutfirst onPATHMutants and the test each one turns red
Each mutant is the head's
test.ymlwith one edit that applies exactly once.m1tom6are the six mutants of #560's review.m7tom12andL1toL12are mine from the first round.X1toX15came from the independent review of the first head,X16toX23are mine, andY1toY8came from the independent review of the second head.cache_key:linecache_save:linecache_save: ${{ github.event_name != 'pull_request' }}env:block withMISE_HTTP_RETRIEShashFiles('mise.lock', 'mise.toml')restore-keysto the stepMISE_HTTP_RETRIES: "5"MISE_HTTP_RETRIES: "six"-{{file_hash}}to the key{{default}}-${{ hashFiles('mise.lock') }}actions/cache@step backif:if: github.event_name != 'pull_request' && ...if: github.event_name == 'push'(no miss check)mise.lockmise.lockidactions/cache/save@v5actions/cache/restore@v5cache_saveonly in a commentSetup miseSet up misetests (no step is named ...)if: ... cache-hit == 'false'if: always() && ...actions/cache@step for./.luarocksrestore-keyson the rock restore{{version}}cache_save: "true"hashFiles('mise.lock')without${{ }}MISE_HTTP_RETRIESmoved underwith:cache_savemoved underenv:cache_keymoved underenv:actions/cache@step for the absolute path of.luarocksactions/cache@step with.luarocksin a multi-linepathactions/cache/save@step for.luarocks./.luarocks./.luarocksactions/cache@step for.luarockswith a double-quoteduses:-${{ github.run_id }}added to the mise key".luarocks"actions/cache@step for~/.luarocksMISE_HTTP_RETRIESset in the job'senv:instead of the step'sactions/cache@step for.luarockswith a single-quoteduses:Green on purpose:
m12,X8,X19andX23are harmless edits. Green because the spec does not check it:X3(a secondcache_save: truekey:actionlintrejects a duplicate key, andmise run format-checkrunsactionlint),X10(the save step moved after the tests),Y3(a secondjdx/mise-actionstep with the default cache settings) andY4(-${{ github.sha }}added to the rock key).Y5,Y6andY7are harmless edits that the spec fails on purpose.actionlintexits non-zero on L8 and L9 (an undefined step id) and X3, andzizmorexits 14 on L10 and L11 (an unpinned action). Both exit 0 on every other mutant.Not covered
if:as an exact string, so a reformatted but equal expression fails it on purpose: edit the spec with the workflow. It does not check that the save step stays right aftermise run setup(X10).Y5), a cache step for~/.luarocks(Y6),MISE_HTTP_RETRIESset in the job'senv:(Y7), and thecache_keyvalue in YAML quotes.uses:only as a plain key. Three spellings of a second.luarockscache step are not seen: a quoted key ("uses": actions/cache@…), which actionlint reads asuses, a step in YAML flow style (- { uses: actions/cache@…, with: { path: .luarocks, key: … } }), and an owner or repository name in other letter case (Actions/cache@…), which GitHub resolves to the same action. A follow-up PR in this run lets the spec accept an optional quote around the key and match the action name in any letter case.jdx/mise-actionstep with the default cache settings (Y3) would save the tool cache on a pull request, and the spec does not look for it.hashFiles('mise.toml'), so text that changes on each run, such as-${{ github.sha }}(Y4), keeps the spec green and the cache never hits..luarocks(a glob such as.luaroc*, or a parent folder) is not seen as a rock cache step.mise.toml(a task edit included) changes the rock cache key, misses, and now rebuilds the rocks on every one of its runs, because a PR no longer saves. Before, its first run saved and later runs hit.mise run setuptook 64 s on one host and 195 s on another. A PR that leavesmise.tomlalone restores main's entry as before.mise run setup, not at the end of the job, so a later red step does not stop the save. The rock tree is complete at that point, and a failedsetupfails the job before the save step.covered-lockstill caches the TLC jar with plainactions/cache.check.shchecks the sha256 of a restored jar on every run and exits 2 before TLC runs on a mismatch. I left it alone.git archive:.gitattributesexport-ignores.github. The failure message says so. A checkout or agit worktreehas the file.Generated with
xum• Model:anthropic:claude-sonnet-5-5• Thinking:xhigh