Skip to content

Drive Dependabot relock through check: --no-onboard gate, --no-narrow, re-pin write fixes - #36

Closed
nodeselector wants to merge 16 commits into
mainfrom
nodeselector/build-update-command
Closed

Drive Dependabot relock through check: --no-onboard gate, --no-narrow, re-pin write fixes#36
nodeselector wants to merge 16 commits into
mainfrom
nodeselector/build-update-command

Conversation

@nodeselector

@nodeselector nodeselector commented Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Why

Dependabot-core needs to re-pin a repo's actions.lock after it rewrites a workflow's uses: ref (e.g. @v4 -> @v5). The design splits cleanly: dependabot-core owns the YAML uses: rewrite, then calls check to re-pin the lock to match. Driving the relock through check (rather than a single-target command) means it reads each workflow's actual ref, so divergent per-workflow refs survive as individual re-pins instead of being flattened to one floating target. This PR makes check honor that contract and fixes two re-pin write-path bugs found while validating it end-to-end against the real binary.

What changed

--no-onboard is now a real gate on check (was a documented no-op). It refuses to onboard new workflows or new actions: a uses: ref with no lockfile entry for its own workflow is rewritten to an onboarding-required finding and skipped, instead of being silently added. A tracked action whose ref was bumped still re-pins (ref-changed), which is the case every Dependabot bump hits. Partial success holds: tracked workflows re-pin and save atomically, refused ones surface as findings, and the run exits 1 with well-formed JSON (not a tool failure).

--no-narrow rides along on check so a chosen ref precision (e.g. @v6) is not silently narrowed to @v6.2.1.

Corrupt-lockfile recovery. A corrupt lock fails loud (exit 2, empty stdout) in CI / non-interactive / read-only paths instead of being silently overwritten, and offers an interactive delete-and-recreate when a TTY is present.

Two re-pin write-path fixes found via credentialed end-to-end runs:

  • Branch dropped on unchanged pins. The lockfile read path drops branch/tag, so a carried verified dep reached State.Set branchless and the strict branch guard rejected the whole write (branch is required in lockfile metadata). Set now reuses the branch already recorded on disk for an unchanged pin; only a genuinely new pin with no branch errors.
  • Stale pins never pruned. A re-pin carried every inventory entry forward, so a version bump left the old pin behind (lock grew unbounded, not idempotent, and the old SHA stayed authorized for a workflow that no longer used it). planWorkflow now prunes inventory entries matching a stale finding before building verified entries; the existing GC then drops the orphaned dependencies[] entry.

Notes for reviewers

  • The onboarding gate is purely subtractive at the command layer (it removes refused refs from the plan); the resolve/pin engine needs no NoOnboard plumbing.
  • onboarding-required is severity:error -> valid:false -> exit 1, but the run still saves the onboarded subset. The dependency field on those findings is always present and always owner/repo@ref (no :sha), which the consumer keys on.
  • Stale pruning is per-workflow; the GC only drops a dependencies[] entry when no workflow references it, so a cross-workflow shared-version bump prunes per workflow rather than globally. Intended.
  • Validated end-to-end against the real linux/arm64 binary and the full Ruby CliEngine#relock (convergent, idempotent, clean read-back). Full Go suite, go vet, and gofmt are green.

nodeselector and others added 15 commits June 9, 2026 10:25
@nodeselector
feat(update): add targeted relock subcommand
64504e3 
Add a new `update` cobra subcommand that relocks a single targeted action
(`--action <owner/repo>@<ref>`) across a repo's workflows + lockfile, driven
by Dependabot via capture3. Built fresh on the existing resolve/pin/lockfile
engine that powers `check`.

Behavior:
- Resolves only the target action's bumped ref and its transitive subtree;
  untargeted deps keep their existing pins verbatim (closure splice).
- `--write` mutates the working tree (YAML `uses:` ref + lockfile pin),
  byte-deterministic and working-tree-only. Without it, computes the diff.
- Refuses to onboard new workflows unconditionally; un-onboarded targeted
  workflows are skipped with an `onboarding-required` finding (partial
  success). `--no-onboard`/`--no-interactive` accepted for Dependabot
  compatibility.
- Emits three always-on arrays (`updated`, `workflows`, `findings`) plus
  `cli_version`, `lockfile_version`, `valid`. Tri-state exit codes reuse
  `check`'s errSilent: 0 clean, 1 blocking findings (stdout still valid
  JSON), 2 tool failure.
- Impostor target (SHA on no branch) is a blocking finding, not a failure.
- Mixed-case YAML refs preserve author casing while keying the lockfile on
  the canonical lowercase NWO.

Tests: pin unit tests, lockfile closure helper, and 7 httpmock integration
tests (clean relock, onboarding partial success, no-op, dry-run, missing
flag, impostor, mixed-case) including raw-JSON contract assertions.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
test(update): lock in diff-minimal lockfile write guarantees
88eaa42 
Validate the consumer's diff-hygiene requirement without changing the writer.
The single canonical writer (marshalDeterministic via Save) already produces a
minimal diff on canonical input — a targeted bump only touches the changed pin
and its changed transitive closure.

- internal/lockfile: TestState_BumpYieldsMinimalDiff (untouched entries stay
  byte-identical across a one-dep bump), TestState_SaveFormatIsStable (exact-bytes
  golden guarding canonical-form drift), TestState_BumpTransitiveRemoval (orphan
  GC'd, cross-workflow shared transitive preserved byte-identically, dropped
  subgraph fully removed).
- internal/pin: TestSpliceClosure_SharedTransitiveExistingPinWins (a moved-tag
  shared transitive keeps its existing pin — one stable entry per ref, no churn
  of untargeted deps).
- cmd/gh-actions-pin: TestUpdateCommand_BumpPullsInNewTransitive (end-to-end add
  of a new transitive; transitive add does not create a second updated[] entry).

No production change: heal-mode full regen is kept (diff-minimal for canonical
lockfiles; older non-canonical lockfiles get a one-time healing reformat), and
the targeted-subtree resolve is unchanged.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
docs(update): correct --json help to match always-on payload
a6c836a 
The long help claimed the selector gates the updated diff, but
WriteUpdateJSON emits all three arrays plus version/valid fields
unconditionally. Make the docs honest: the selector is accepted for
symmetry with check but gates nothing.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
Harden update: atomic write, relock invariant, audit polish
fa118e1 
Apply the Tier 1 + Tier 2 fixes from the six-reviewer audit of the `update`
command. No change to the three-array JSON contract Dependabot consumes.

Tier 1 (correctness):
- Enforce the relock invariant. PlanUpdate now refuses a SHA-shaped
  --action ref up front (would write a sha-as-ref instead of pinning) and
  verifies each spliced closure pins the target at the new ref + resolved
  SHA, surfacing a blocking finding (per spec #9) rather than a silent
  exit-0 no-op. Changes are recorded only after a workflow passes verify, so
  updated[] never advertises a bump that was skipped.
- Make CommitUpdate atomic w.r.t. the lockfile. Workflow YAML is staged to
  sibling temp files, the lockfile is saved (atomic temp+rename), then the
  temps are promoted with rename. No workflow file is mutated until the
  lockfile is durable, so a staging/save failure leaves the tree untouched.
  Promotion preserves the destination's mode and follows symlinks, so a 0600
  or symlinked workflow keeps its identity. (Moved to update_commit.go.)
- Add Remediation to impostorFinding; reword onboarding-required to "pin
  this workflow first"; correct the OnboardingRequired category comment to
  say refusal is unconditional (--no-onboard does not gate it).
- Sort findings[] deterministically at the JSON boundary.

Tier 2 (polish):
- Case-fold the existing-SHA lookup so non-canonical NWO casing can't read
  as a phantom change.
- workflows[] lists only files whose YAML was actually rewritten (a
  lockfile-only re-pin is omitted); invisible to Dependabot's new-ref path.
- Fix user-facing text: exit-status JSON caveat, --json usage string, and a
  distinct "no changes applied" summary when a run is blocked vs a clean
  no-op. Surface ExtractActionRefs warnings on stderr in non-JSON mode.
- Move renderUpdateSummary into format as PresentUpdateSummary; replace
  containsStr with slices.Contains; drop the unused fieldsCSV param from
  WriteUpdateJSON.

Tests: relock-invariant (sha-shaped target is blocking, no write), atomic
write-failure consistency, mode preservation, and verifyTargetPin/looksLikeSHA
units.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
feat(outdated): read-only version discovery verb
8ba6513 
Add `gh-actions-pin outdated`, a read-only companion to `update`. For each
action recorded in the lockfile it reports the newest ref available,
preserving the precision of the current ref (major float v5 -> highest
major-form v6; full pin v5.1.0 -> highest full v6.1.2), allowing cross-major
upgrades, and offering strictly-greater versions only. The picker mirrors
dependabot-core's github_actions UpdateChecker so an offered ref matches the
ref an eventual Dependabot PR would land.

Never writes. current_ref/current_sha come straight from the lock entry and
are never re-resolved (keeps the CLI's report aligned with the recorded lock
and preserves churn-free convergence with Dependabot). Only network work is
tag listing; available_sha is the tag's peeled commit SHA.

Output: always-on available_updates[] (bare SHAs, omitempty current_sha,
precision enum major|minor|full|sha) alongside cli_version/lockfile_version.
Tri-state exit optimized for a dev terminal: 1 = updates available (npm
`outdated` convention, via errSilent), 0 = clean, 2 = tool failure. Tokenless
fails at tag-list with exit 2 + empty stdout, matching check/update.

Pagination is a parity requirement, not a nicety: core enumerates all tags via
git ls-remote, so ghapi.ListTags now pages (per_page=100, 50-page cap) to avoid
under-offering on >100-tag repos.

- internal/discover: pure picker engine (precision classify, strict-greater,
  same-precision-then-coarser fallback) + table tests.
- internal/ghapi: paginate ListTags + pagination test.
- cmd/gh-actions-pin: outdated command, format.AvailableUpdate +
  WriteOutdatedJSON, PresentOutdatedSummary, root registration, command-level
  golden tests (major-float offer, full-pin offer, up-to-date no-op, bad
  selector usage error).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
feat(update): fold version discovery into an interactive picker
ab06d1e 
Replace the standalone read-only `outdated` verb with an interactive
version picker on `update`, and split the relock flags.

- `--action owner/repo@<current>` now names the dependency and its
  CURRENT pinned ref (the precision anchor), and `--target <next>` is
  the destination ref. This inverts the old meaning where `--action`'s
  ref was the destination.
- With no `--target` on an interactive terminal, `update` lists the
  newer versions available for the action (semver-desc, non-semver tags
  in release order) and relocks to the picked ref. Non-interactive with
  no `--target` is a usage failure (exit 2), never a hang or auto-pick.
- `--write` is removed: `update` writes by default. The picker UI renders
  to stderr so `update --json` stdout stays pure JSON.

Engine, JSON output (updated/workflows/findings), and tri-state exit
codes are unchanged. The `internal/discover` engine is repurposed from a
single-offer picker to `Candidates`, a ranked next-version list; the
paginated tag listing is retained for picker parity.

The relock argv for downstream callers moves from
`update --action name@<target> --write` to
`update --action name@<current> --target <next>`.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
feat(update): default the version picker to dependabot-core's pick
d708676 
The interactive picker defaulted to the highest semver tag overall, so a
major-form float (e.g. @v5) defaulted to the highest full tag (v6.1.2)
when dependabot-core would land the highest major-FORM tag (v6). A dev
hitting Enter then wrote a ref that core's next run would flip back —
avoidable churn.

Add discover.CorePick, mirroring core's GitCommitChecker two-step
precision rule:
  1. among candidates of the same precision (dot-segment count) as the
     current ref, take the semver-max;
  2. only if that is empty, take the global semver-max over candidates of
     lower-or-equal precision.
Candidates are gated to core-versions: same prefix, v-prefixed or dotted
(core's VERSION_REGEX rejects bare numerics like "5"), strictly greater,
stable unless the current ref is itself a prerelease.

pickTargetRef now seeds the picker's default with CorePick so the default
choice equals the eventual-PR ref; the full candidate list is unchanged,
so a dev can still deliberately pick a higher-precision tag. The relock
contract and JSON output are untouched (the VM path always passes
--target and never reaches the picker).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
fix(lockfile): fail loud on corrupt lockfile instead of silent overwrite
9997e73 
An unreadable .github/workflows/actions.lock (corrupt YAML, unknown
field, or a dependency entry missing a required key) was silently
treated as empty and overwritten on the next save, degrading every
workflow to onboarding-required/not-pinned with no diagnostic.

LoadState now returns ErrCorruptLockfile instead of swallowing the
parse error. A recovery policy in the command layer decides what to do:
interactive check fix-mode offers to delete and recreate the lockfile;
non-interactive/CI/--no-interactive/read-only (--no-fix) and update all
fail with a clear, actionable error (exit 2, empty stdout, file kept).
update never offers deletion because it only relocks already-onboarded
workflows, so a delete would make it a silent no-op.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
test(recovery): prove corrupt-lock delete-and-recreate re-pins end-to…
22420ac 
…-end

Add a confirmFactoryHook seam so command tests can drive the interactive
delete-and-recreate path without a TTY, and cover it end-to-end:

- confirm-yes: corrupt lock is deleted and the workflow is fully re-pinned,
  writing a fresh valid lockfile with the real resolved SHA + owner/repo IDs
  (not the corrupt fake), exit 0.
- confirm-no: run fails loudly and the unreadable lockfile is left
  byte-identical.

This closes the duck-flagged gap: the recovery policy was only unit-tested in
isolation; now the whole check fix-mode recovery -> re-pin -> write path is
exercised against the httpmock harness. Confirms the recovered lock is rebuilt
correctly (the reported empty-record/no-lock anomaly does not reproduce under
deterministic conditions).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
refactor: drop the update command; keep corrupt-lock recovery
0dd389e 
dependabot-core will own workflow-YAML rewriting and call `check` to
re-pin, so the standalone `update` relock verb and its interactive
version picker are removed. Deletes update.go, the pin/update engine,
internal/discover, and the update JSON/terminal surfaces.

Preserved (already part of the validated contract): the corrupt-lockfile
recovery path (ErrCorruptLockfile, lockrecovery), State.HasWorkflow, and
the OnboardingRequired finding — the last two now back the upcoming
`check --no-onboard` gate. --no-onboard/--no-interactive stay registered
as persistent flags on the root check invocation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
feat: add --no-narrow flag and respect locked ref precision
7040e48 
Add --no-narrow flag to preserve mutable version refs (e.g. v4) in the
lock comment instead of narrowing them to full patch tags (v4.2.1).

Once a dep is locked with an imprecise tag, subsequent re-pins (e.g. on
ref-moved) respect that choice by checking the existing lockfile for
mutable refs before narrowing. This makes the precision sticky without
requiring --no-narrow on every run.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
feat(check): make --no-onboard refuse onboarding new workflows/actions
7fe97a9 
Wire --no-onboard to actually gate onboarding on the `check` path (it was
previously accepted but ignored). After diagnosis, rewrite each not-pinned
finding to onboarding-required and drop its ref from the workflow's action
set so Plan/Commit never resolves or pins it. Already-tracked actions whose
ref was bumped surface as ref-changed and still re-pin — the onboarding
boundary is per workflow, so the existing not-pinned/ref-changed split
already distinguishes "new" from "bumped".

Refused entries force a blocking (errSilent → exit 1) run with well-formed
JSON on stdout; they never escalate to a tool failure. Render
onboarding-required as an alerted category in terminal output.

Drop the now-unused State.HasWorkflow (its doc referenced the removed
`upgrade` command).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
fix(lockfile): preserve branch/tag for unchanged pins on re-pin write
bac95e9 
A ref-bump or newly-added action in an already-tracked workflow failed to
write with "branch is required in lockfile metadata". The lockfile read
path (parserlock.Pin) drops branch/tag, so a carried Verified dep arrives
at State.Set branchless. Set rejected the whole write instead of falling
back to the branch already recorded on disk for that unchanged pin.

Set now preserves Branch/Tag from the existing on-disk entry when an
unchanged pin arrives branchless, and only errors when a genuinely new
pin has no branch. This unblocks the consumer's check-based re-pin happy
path (Dependabot rewrites uses: vN->vN+1, then runs check to re-pin).

Adds a lockfile unit regression test and a command-level
--no-onboard --no-narrow ref-bump write test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
fix(pin): prune stale pins on re-pin so the lock converges
d5fac8a 
A fix-mode re-pin carried every inventory entry forward as a Verified
pin, including stale ones (lockfile pins the workflow no longer references
via uses:). So a Dependabot version bump left the prior pin behind: the
workflow listed BOTH the old and new ref, the orphaned dependency entry
never GC'd, and the run was not idempotent-convergent. check even emitted
the stale finding, then ignored it.

planWorkflow now prunes inventory entries matching a stale finding before
building Verified entries, so the orphaned pin drops from workflows[path]
and Save's GC removes the now-unreferenced dependencies[] entry. This also
closes a supply-chain gap: the old SHA is no longer authorized for a
workflow that no longer uses it.

The combined --no-onboard --no-narrow ref-bump write test now asserts the
stale ref and orphaned SHA are gone after the re-pin.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@nodeselector
style: trim added comments to two lines max
829b6b5 
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 10, 2026 05:52
Copilot AI reviewed Jun 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Human review recommended

It changes core lockfile read/write semantics and check command behavior (including recovery flows) in ways that benefit from final human validation across real-world repos and CI environments.

Pull request overview

This PR updates gh-actions-pin to support Dependabot-driven “relock” flows by routing re-pinning through check (instead of a dedicated update subcommand), adding operational gates (--no-onboard, --no-narrow), and fixing correctness issues in the lockfile read/write paths so relocks converge and remain idempotent.

Changes:

  • Make check suitable for Dependabot relock: --no-onboard now enforces a hard “no new entries” gate, and --no-narrow preserves mutable tag precision.
  • Fix lockfile robustness and convergence: surface corrupt lockfiles explicitly (with controlled recovery), preserve branch/tag metadata for unchanged pins, and prune stale pins during re-pin planning.
  • Add/expand tests covering onboarding refusal behavior, corrupt lockfile recovery behavior, branch preservation, minimal-diff guarantees, and transitive GC behavior.
File summaries
File Description
internal/pipeline/checks/category.go Updates schema/category docs for onboarding-required under check --no-onboard.
internal/pin/plan.go Adds NoNarrow, prunes stale inventory during planning, and respects prior mutable-ref precision.
internal/lockfile/state.go Introduces ErrCorruptLockfile, stops swallowing parse errors, and preserves branch/tag metadata on unchanged pins.
internal/lockfile/state_test.go Adds coverage for corrupt lockfile surfacing, preserved branch/tag on unchanged pins, GC/minimal-diff behaviors, and writer stability.
cmd/gh-actions-pin/root.go Adds persistent --no-onboard / --no-interactive flags and corrupt-lockfile recovery wiring in newRun.
cmd/gh-actions-pin/check.go Implements --no-narrow, applies --no-onboard gating, and integrates lockfile recovery policy into check.
cmd/gh-actions-pin/pin_summary.go Extends terminal summary behavior to account for onboarding refusals.
cmd/gh-actions-pin/onboard_gate.go Adds the --no-onboard gate implementation that rewrites findings and drops refused refs from planning.
cmd/gh-actions-pin/lockrecovery.go Adds interactive/non-interactive corrupt lockfile recovery policy.
cmd/gh-actions-pin/lockrecovery_test.go Tests the recovery policy behavior across interactive/headless/read-only cases.
cmd/gh-actions-pin/no_onboard_command_test.go Tests --no-onboard semantics for new actions, new workflows, and ref bumps (including write behavior).
cmd/gh-actions-pin/corrupt_lock_command_test.go Tests corrupt lockfile behavior for check in read-only and fix modes, including delete-and-recreate confirmation.
cmd/gh-actions-pin/format/terminal.go Includes onboarding-required in error rendering and alert classification.
go.mod Adds indirect module requirements needed by new/updated CLI/test dependencies.
go.sum Updates module checksums corresponding to the new indirect requirements.

Copilot's findings

  • Files reviewed: 14/15 changed files
  • Comments generated: 0

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nodeselector nodeselector mentioned this pull request Jun 10, 2026
Merged
@nodeselector
fix(pin): retain on-disk impostor pins on co-located re-pin
7162677 
Fix-mode dropped an impostor-flagged dep from a workflow's pin set whenever
a co-located bump forced that workflow to be rewritten, shrinking the list
toward [] and trivially passing runner enforcement. Re-add the existing
on-disk pin (kept direct) so the blocking finding + exit 1 still stand but
the pin is never silently lost.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nodeselector