Repository navigation
feat(pin): add --check to pin and upgrade - #116
Merged
Merged
Conversation
A dry run named each action in the past tense over a file it had been told not to write: ``` pinned actions/checkout@v4.1.0 → 8ade135a41bc ``` Nothing was on disk. The line now reads "would pin", and the upgrade side "would upgrade". Cosmetic on its own, and not under the --check the next commit adds: those per-action lines are the whole of what a reader opens a failed step to see, and every one of them claimed the fix was already in. `internal/pin/dry_run_verb_test.go` covers both directions, with a control that a real run still reports what it did.
`--dry-run` works: both commands resolve every action, report what they found and write nothing. What it cannot do is fail. The exit code is 0 on a directory where every action follows a moving tag and 0 on one already pinned, so a CI step running `cinzel github pin --dry-run` passes either way. This is `pin-and-upgrade-reported-a-failure-at-exit-zero.md` one step on. That note made a failed resolve exit non-zero. An action that resolved, in a run told not to write, stayed at 0 — which is the case a pinning check is about. ## Fix `--check` on both, the way `fmt --check` and `diff` already work: ```sh cinzel github pin --check # fails if anything still follows a tag cinzel github upgrade --check # fails if anything has a newer release ``` It implies `--dry-run`. A check that pinned what it was asked to look for would pass on its own side effect, and the second run of a two-run step would find the file clean because the first run cleaned it. The summaries already count what happened, so the finding is a count that was there all along, read the other way round: an action that had to be resolved was not pinned when the run started, and nothing was written, so it still is not. A failed resolve outranks it. A run that could not reach an action does not know whether it is pinned, so reporting it as unpinned is a guess. `--dry-run` alone is untouched and still exits 0. The summary line also read "1 pinned" over a file left alone, and now counts what is not pinned. ## Tests `internal/command/pin_check_test.go`. Resolving a tag is an API request, so the CLI-level cases use what answers offline: an action on a full SHA is skipped without a request, a local action fails without one. The decision is in the summary, so the finding is tested there directly. `TestPinCheckWritesNothingToAFileItWouldPin` needs a run that genuinely resolves — a run that resolved nothing leaves the file alone too and proves nothing — so it seeds the 24h pin cache, and asserts on the report as well as the file: if the key stops matching, the test fails rather than passing on a resolution it never made. Upgrade has no offline path that rewrites a file, so that case covers pin only. Each branch was checked by reverting it and watching a test go red. Verified against the real GitHub API in both directions, files compared byte for byte before and after.
Go test coverage: 78.9% for commit 88f1eef
|
Five actions in cinzel/steps.hcl, with .github/workflows regenerated from it: actions/create-github-app-token v3.2.0 → v3.3.0 jdx/mise-action v4.3.0 → v5.2.6 stefanzweifel/git-auto-commit-ac… v7.2.0 → v7.2.1 ietf-tools/semver-action (unpinned) → v1.12.0 actions/configure-pages v5 → v6.0.0 semver-action carried a bare SHA with no tag comment, so the version it was on could not be read off the line. It now says v1.12.0. Two majors: mise-action and configure-pages. CI on this PR is what exercises them. Each SHA was checked against the tag its comment claims, dereferencing annotated tags. `mise run drift` reports 4 checked, 0 differ, so the HCL and the generated YAML are in step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
--dry-runworks — both commands resolve every action, report, and write nothing. What it cannot do is fail, so nothing in CI can gate on it:--checkimplies--dry-run: a check that pinned what it was asked to look for would pass on its own side effect. A failed resolve outranks the finding, since a run that could not reach an action does not know its state.--dry-runalone is untouched and still exits 0.A second commit fixes the wording. Under
--checkthe per-action lines are what a reader opens the failed step to see, and they claimed the fix was already in:Evidence
Against the real GitHub API, on a scratch copy of
cinzel/:Before — a pinned tree and an unpinned one are indistinguishable:
After:
pin --check, unpinned presentpin --check, clean treeupgrade --check, 2 outdatedupgrade --check --parse(no YAML written)upgrade --dry-run(unchanged)Files byte-identical after every
--checkrun. Each branch was checked by reverting it and watching a test go red.go test ./...green,gofmtandgo vetclean.Also here: an action bump
997836fupgrades the five actions this repo's own CI runs on, incinzel/steps.hclwith.github/workflowsregenerated from it:Two majors —
mise-actionandconfigure-pages. A red check here is the bump, not the feature. Every SHA was verified against the tag its comment claims.Merge Danger
Door: two-way — new flag, defaulted off.
Blast Radius: output wording, plus this repo's own CI.
No existing invocation changes its exit code. Anything grepping the dry-run report for
^pinnedor^upgradednow readswould pin/would upgrade. The action bump is the real risk, and it is its own commit.