Repository navigation
fix(lock): close-lock keeps a moved tag as a leaf's ref: when no current tag names the pin - #1323
Merged
Merged
Conversation
…ent tag names the pin close-lock refused to write a leaf record for a SHA-pinned action when no current tag pointed at that SHA (SKIP, exit 3). dtolnay/rust-toolchain moves `v1`, so old pins such as @e2a55d2 (standards@75bd5994 mirror-reusable.yml) and @02cb101 (panic-attack@b689a74 scan-and-report.yml) became unclosable, and every caller's actions.lock with them (measured on hyperpolymath/echidna#446, run 39423933176). GitHub accepts a record whose ref: names a tag that has since moved. Measured on standards run 39286638218 (2026-10-19 16:36 UTC): the runner used the lockfile, downloaded dtolnay/rust-toolchain@e2a55d2 from a record with ref: 'v1', and the step passed. v1 was already 683b588 by then (Dependabot moved standards to it at 14:45 UTC, e926751). Fallback, tried only when the tag listing succeeded and found nothing, in order: 1. the same key's record in the lock as it stood before gh actions-lock (new --prior FILE; regen_lock now snapshots and passes it), used only when its commit is the key SHA and its owner/repo ids match the API; 2. the `# <tag>` comment at a call site of that SHA (this repository's workflows and the called workflows close-lock already reads). Either candidate is written only if it is a tag now (commits/tags/<t>) and its commit descends from the pin (compare status ahead|identical). Branch names (`# stable`), deleted tags, self-SHA refs (standards#1271) and tags on another line are refused, and the key SKIPs with exit 3 as before. Tests: close-lock 34 -> 53 offline cases (both sources, and every refusal as a planted positive). regen-dependabot-locks 59 -> 61 (close-lock gets the pre-regeneration lock byte for byte; nothing is left in the checkout). lock-selfcheck 9/9 unchanged. All pass under mawk and gawk; nine guard mutants are each killed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHfuPCv1pEiqkQGG5DQASb
Contributor
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 37 minutes. View limit details
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. Comment |
hyperpolymath
marked this pull request as ready for review
October 20, 2026 14:14
Codex Review SummaryThis 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. |
Contributor
|
🟢 conformant · commit
K9 live corpus (L0–L2): 2 finding(s) / scope notes
Scope: DEED syntax and K9 contracts L0–L2. L3 signature verification and routed coordination dialects are not certified. Green is limited to this scope. Orange is missing evidence; grey is an explicit scope exclusion. Logs and report.json are in the dated conformance-evidence artifact on the linked run. |
|
hyperpolymath
enabled auto-merge (squash)
October 20, 2026 14:31
hyperpolymath
disabled auto-merge
October 20, 2026 14:34
hyperpolymath
enabled auto-merge (squash)
October 20, 2026 14:35
hyperpolymath
added a commit
that referenced
this pull request
Oct 27, 2026
…own lock Follow-up to #1323. Its two sources do not close callers of standards@75bd5994: mirror-reusable.yml pins dtolnay/rust-toolchain@e2a55d2 with the comment `# stable`, which is a branch and is correctly refused, and a caller's lock never held that record. The record does exist in standards@75bd5994's own actions.lock, as ref: 'v1'. That is the lock GitHub validates the called workflow against, and at a SHA pin it cannot change. New source, tried between the prior record and the call-site comment: for each reusable whose external uses: name the leaf key, the record for that key in the called repository's .github/workflows/actions.lock at the called ref. close-lock reads that lock once per called owner/repo@ref, while it reads the called workflow. The checks are unchanged and shared with the prior record (try_record): commit equals the pin, owner/repo ids match the API, and the tag exists now and descends from the pin. A missing or malformed callee lock is not a source. Tests: close-lock 53 -> 59 offline cases. They cover the 75bd599 shape, fall-through from a refused prior record, and refusals for a wrong commit, a callee that does not use the key, and a lock with no dependencies: section. All eight lock suites pass under mawk and gawk. 11 guard mutants are each killed. shellcheck is clean. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JHfuPCv1pEiqkQGG5DQASb
hyperpolymath
added a commit
that referenced
this pull request
Oct 27, 2026
…own lock (#1350) ## Summary Follow-up to #1323, which added two fallbacks for a leaf whose tag has moved: the record from the lock before regeneration, and the call-site `# <tag>` comment. Neither closes a caller of **standards@75bd599**: - `mirror-reusable.yml` pins `dtolnay/rust-toolchain@e2a55d2…` with the comment `# stable`. In dtolnay, `stable` is a branch, so #1323 correctly refuses it. - A caller's lock never held that record. echidna@c020b63 is an example. The record does exist in standards@75bd5994's **own** `actions.lock`, as `ref: 'v1'`. That is the lock GitHub validates the called workflow against, and at a SHA pin it cannot change. ## Change A third source, tried after the prior record and before the call-site comment. - **What it is.** For each reusable workflow whose external `uses:` names the leaf key, close-lock looks for that key's record in the called repository's `.github/workflows/actions.lock`, at the ref it is called at. - **Cost.** close-lock already fetches the called workflow; the callee's lock is read once per called `owner/repo@ref` alongside it. - **Checks.** The same checks as the prior record, through one shared helper (`try_record`): - `commit` equals the pin; - `owner_id` and `repo_id` match what the API returns now; - the `ref` is a tag now (`commits/tags/<t>`) whose commit descends from the pin (compare `ahead` or `identical`). - **Missing or malformed lock.** Not a source. `lock_record` reads records only under a `dependencies:` section. - **Borrowing.** Only callees that actually use the key are consulted. A matching record in an unrelated callee's lock is not borrowed. ## Testing Offline, locally: - **`close-lock-test.sh`: 53 → 59 cases, 0 failed.** The new cases cover: - the 75bd599 shape: a `# stable` comment, with the callee's lock carrying `ref: 'v1'`; - fall-through from a refused prior record to the callee's lock; - planted refusals: a callee record for another commit (the SKIP line names every refused candidate), a callee that doesn't use the key, and a callee lock with no `dependencies:` section. - **Other suites, unchanged and passing under both mawk and gawk:** regen-dependabot-locks (61), lock-selfcheck (9, with mikefarah yq v4), check-lock-sync (13), relock-sha-keys (12), complete-job-refs (20), validate-actions-lock (7), relock-standards-pin. - **Mutants:** removing any one of 11 guards turns at least one case red. The guards are: - callee source, callee-uses-key check; - commit check, id check; - ancestry, tag existence, SHA-name check; - no fallback on a failed tag listing; - prior source, `dependencies:` scoping, comment source. - **shellcheck `-S warning`:** clean. **Not yet done:** a live run against the real API. The next step is a scratch CI run on echidna's pre-#446 tree, where both old dtolnay pins should close with 0 dangling refs and the workflows should start. --- _Generated by [Claude Code](https://claude.ai/code/session_01JHfuPCv1pEiqkQGG5DQASb)_ Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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
scripts/close-lock.shwon't write a leaf record for a SHA-pinned action when no current tag points at that SHA. It printsSKIP … (cannot resolve a symbolic tag for leaf SHA)and exits 3.dtolnay/rust-toolchainmovesv1, so old pins can no longer be closed. Two examples:@e2a55d2…(standards@75bd5994mirror-reusable.yml) and@02cb101…(panic-attack@b689a74scan-and-report.yml). Every caller lock that reaches those pins is stuck with them (hyperpolymath/echidna#446, run 39423933176).This PR adds a fail-closed fallback. It does not bring back self-SHA leaf refs (cf8f912, standards#1271).
Measurement: GitHub accepts a
ref:naming a tag that has since movedRun 39286638218, job
mirror / mirror-radicle, standards@333b06a0, 2026-10-19 16:36 UTC. The runner log shows:The record it used was
ref: 'v1'. "Setup Rust" passed. By thenv1was already683b588: Dependabot moved standards' own pin to it at 14:45 UTC that day (e926751, #1300). Soref:is not re-resolved against the tag's current target.Change
Fallback, tried only when the tag listing succeeded and found no tag, in this order:
gh actions-lockrewrote it. close-lock reads it from a new--prior FILEoption.regen_lockinregen-dependabot-locks.shnow snapshots the lock before the tool runs and passes it in. The record is reused only when itscommitis the key SHA and itsowner_id/repo_idequal what the API returns now.# <tag>comment on auses:of that SHA. close-lock looks in this repository's workflows and in the called workflows it already reads.A candidate is written only when both of these hold:
commits/tags/<t>), andcompare/<sha>...<tagsha>reportsaheadoridentical), which means the tag moved forward past the pin.Anything else is refused, and the key SKIPs with exit 3 as before: a branch name such as
# stable, a deleted tag, a self-SHA ref, or a tag on another line. The SKIP line now lists why each candidate was refused.Testing
Offline, locally:
close-lock-test.shregen-dependabot-locks-test.shlock-selfcheck-test.showner_id, self-SHA (refused by name before any lookup), a tag that doesn't exist,# stable, a tag that doesn't descend from the pin, a comment on another action's pin, and a failed tag listing (no fallback on an API error).&& ok || badidiom it already uses.Not yet done: a live run. These lookups can't be made from this session, so the fallback has not run against the real API. Going by the API docs,
commits/tags/<t>and the comparestatusbehave as the stub assumes, but that is unverified until CI runs them.Known limit
This alone does not close echidna@c020b63, the pre-#446 tree. The panic-attack call site reads
@02cb101… # v1, which closes through source 2. But standards@75bd5994's call site reads@e2a55d2… # stable, which is a branch and so correctly refused. Echidna's own lock never held ane2a55d2record. The record does exist in standards@75bd5994's ownactions.lock(ref: 'v1'), which is the lock GitHub validates the callee against. Whether to add that as a third source is an open question for a stacked follow-up.Related
gh actions-lock --verifyv0.2.0 reports job-level reusable records as "Unused lockfile entry". That conflicts with close-lock's closure requirement.update-actions-lock.sh --verify-localalready accepts that coverage. It may be worth an upstream issue on github/gh-actions-lock.Generated by Claude Code