Tag narrowing, dependabot contract, and integration hardening - #48
Merged
Conversation
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>
Narrowing now checks whether a ref is a full semver tag (v4.2.1) rather than just whether it's a mutable semver. This broadens the nudge to cover any non-semver ref (v4, main, arbitrary tags) — pushing users toward precise refs where each tag resolves to exactly one commit. Sticky precision is computed globally across all workflows (not per-WF) to avoid creating duplicate dep entries at different ref granularities for the same NWO. Once a dep is locked imprecisely anywhere, all workflows consistently use that ref. Terminal output: TermWarn nudge listing non-semver deps with guidance. JSON output: info-severity mutable-ref findings injected per workflow. Both suppressed when --no-narrow is set (user explicitly opted out).
Drop the 'mutable' framing — any git ref can be rewritten today, so the term is misleading. VersionRef better describes what we're checking: is the ref a full semver tag (v4.2.1) or not. Renames: MutableRef to VersionRef, mutable-ref to version-ref, prevMutableNWO to prevImpreciseNWO, IsMutable to IsFull.
Bring in the infrastructure Dependabot needs to shell out to gh-actions-pin check --no-onboard --no-narrow --no-interactive: - --no-onboard / --no-interactive persistent flags on root command - onboard gate: rewrites not-pinned → onboarding-required for new entries when --no-onboard is set, so relock never silently adds deps - corrupt lockfile recovery with interactive confirm / CI fail-fast - branch/tag preservation in lockfile Set() for branchless read-path - stale inventory pruning so Plan() skips entries the pipeline dropped - impostor pin retention and commit helpers (pin/commit.go) - OnboardingRequired in terminal error + alerted category lists All existing tests pass; new test files for lock recovery, impostor retention, and lockfile state included.
22 new scenarios across 3 categories: - narrowing (12): default v4→full, minor→patch, branch skip, --no-narrow keeps major/minor, sticky imprecise re-pin, nudge visibility, JSON - onboarding (8): --no-onboard new/tracked/mixed, JSON findings, terminal caution, flag composition with --no-narrow/--no-interactive - lockfile (2): corrupt + --no-fix fails, corrupt + --no-interactive fails
The --no-fix early return path exits before the SSO URL display block, so users never see the actionable authorization link when running read-only. Move the SSO URL surface before the early return so it shows regardless of fix mode. Closes #45
Rename retainImpostorPins → retainUnresolvablePins and broaden the filter to also retain Unresolved entries (not just Investigate + ImpostorCommit). When a 403/SSO or transient error prevents re-resolution, the existing on-disk pin is preserved instead of being silently dropped. New test verifies the Unresolved retention path round-trips through a lockfile write. Closes #46
Change gateNoOnboard to return refused entry labels (e.g. "actions/checkout@v4 in .github/workflows/ci.yml") instead of just a count. renderPinSummary now lists each refused entry so users know exactly what was skipped. Also removes the stale 'live' tag assertion from catalog_test.go (live scenarios were removed earlier). Closes #44
SSO 403 on a previously-pinned action — exercises the not-pinned/error finding path that the dependabot-core dropped-pin guard keys on. Uses the default SSO 403 stub with a pinned_checkout lockfile template. Ref #47
When --no-onboard skips new workflows and no other blocking findings exist, exit 0 instead of 1. Onboarding-required findings are downgraded to info severity (non-blocking) but remain in the JSON output so machine consumers can still observe the skip. This aligns with the dependabot-core consumer contract: dependabot never bootstraps lockfiles (that's the onboarding flow's job), so an incremental run that only sees new/un-onboarded workflows should not block. Mixed runs (onboarding-required + impostor/not-pinned/unresolved) still exit 1 because the blocking findings survive. Closes #43
Skipped scenarios for the dependabot contract category covering impostor-commit and lockfile-forgery findings. Both require multi-phase HTTP stub infra (GraphQL + REST) that doesn't exist yet — marked with skip + needs_stub. Category strings confirmed: impostor-commit, lockfile-forgery. Severity: error. Shape matches existing dbot scenarios.
Add --golden-update [category] to the integration harness. Runs each scenario in the category, captures the literal --json stdout body, and writes it back into catalog.yml under expect.golden_json. This makes the binary the source of truth for the contract — when you change a category, severity, or exit mapping, re-run golden-update and the scenario bodies change in the same PR. Also: - OnboardingRequired now returns IsValid()=true, fixing valid:false in JSON output at exit 0 for --no-onboard runs. - golden_json assertion validates the full JSON body on every run. - Catalog Go structs gain JQCheck, GoldenJSON, and Skip fields. - Fixed greater_than/gt jq assertion mismatch. - Updated stale dbot scenario assertions caught by golden capture: dbot_not_pinned, dbot_multi_workflow, dbot_transient_403_drops_pin.
There was a problem hiding this comment.
⚠️ Not ready to approve
There are correctness issues in new behavior/tests (notably lockfile recovery path selection under GH_ACTIONS_PIN_WORKFLOWS_DIR, sticky narrowing semantics, and scenario exit expectations) that can break intended contract behavior.
Pull request overview
This PR expands gh-actions-pin check to support Dependabot’s relock contract by hardening exit-code semantics, adding tag-narrowing behavior (with opt-out + “sticky precision”), and strengthening integration tests via a scenario catalog with golden-captured JSON bodies.
Changes:
- Add tag narrowing (major/minor → full patch tag at same commit) with
--no-narrow, plus “version-ref” informational nudges and JSON surfacing. - Introduce
--no-onboardgating (refuse onboarding new workflows/actions) and adjust validity semantics so onboarding-required-only runs are non-blocking (exit 0,valid: true). - Add/extend integration scenario infrastructure (jq assertions, golden JSON capture/update flow, richer harness UI) and lockfile corruption recovery behavior.
File summaries
| File | Description |
|---|---|
| test/scenarios/catalog.yml | Adds new scenario categories (narrowing/onboarding/dependabot), new scenarios, and golden JSON contract fixtures. |
| test/scenarios/catalog.go | Extends scenario schema to support skip, jq checks, and embedded golden JSON bodies. |
| test/scenarios/catalog_test.go | Updates tag-based catalog test expectations after removing “live” tagging. |
| test/integration/run.rb | Enhances fixture generation and assertion hydration (jq checks, golden JSON diff), adds lockfile templates. |
| test/integration/harness.rb | Adds golden capture mode, interactive review tooling, PTY prompt automation, diff caching/paging. |
| internal/pipeline/checks/finding.go | Adjusts workflow attention/validity logic to treat version-ref and onboarding-required as non-blocking. |
| internal/pipeline/checks/category.go | Updates onboarding-required definition and adds the new version-ref category. |
| internal/pipeline/checks/category_test.go | Freezes/validates new category string and inconclusive semantics. |
| internal/pin/retain_impostor_test.go | Adds unit tests ensuring unresolved/impostor pins are retained during co-located re-pins. |
| internal/pin/plan.go | Implements tag narrowing + sticky precision logic and prunes stale inventory entries. |
| internal/pin/commit.go | Retains existing pins when a dependency becomes unresolvable during re-resolution. |
| internal/lockfile/state.go | Surfaces corrupt-lockfile errors, preserves branch/tag for unchanged pins, and tightens parsing behavior. |
| internal/lockfile/state_test.go | Adds regression and golden-format tests for lockfile stability, corruption surfacing, and GC behavior. |
| go.mod | Adds new indirect dependencies required by updated runtime/test behavior. |
| go.sum | Updates module checksums for new/updated dependencies. |
| cmd/gh-actions-pin/root.go | Introduces persistent flags and lockfile corruption recovery wiring for root invocations. |
| cmd/gh-actions-pin/pin_summary.go | Adds onboarding refusal summary details and version-ref nudge output. |
| cmd/gh-actions-pin/onboard_gate.go | Implements report rewriting + ref dropping for --no-onboard. |
| cmd/gh-actions-pin/lockrecovery.go | Adds interactive corrupt-lockfile delete-and-recreate policy (TTY/CI aware). |
| cmd/gh-actions-pin/lockrecovery_test.go | Tests lockfile recovery policy behaviors (interactive/non-interactive/read-only). |
| cmd/gh-actions-pin/format/terminal.go | Ensures onboarding-required is surfaced in terminal error summaries/alert logic. |
| cmd/gh-actions-pin/check.go | Wires --no-narrow, --no-onboard, version-ref injection, and read-only SSO URL surfacing. |
| .gitignore | Ignores dist/ output directory. |
Copilot's findings
- Files reviewed: 21/23 changed files
- Comments generated: 7
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.
- plan.go: only track imprecise semver refs (v4, v3.1) in the sticky- precision set, not branch refs like main. Branch refs are a different kind of reference entirely and shouldn't disable narrowing for semver refs of the same NWO. - catalog.yml: fix 4 onboarding scenario exit codes (1→0). Under the Option 2 semantics (#43), onboarding-required findings are info-level and non-blocking — runs with only onboarding-required findings exit 0. Also fix fresh_no_narrow_nudge_suppressed description to match intent. - root.go: thread workflowsDir through corrupt lockfile recovery path. Previously the recovery always pointed at .github/workflows/actions.lock even when GH_ACTIONS_PIN_WORKFLOWS_DIR overrode the load path.
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.
What
Adds tag narrowing (splat → semver precision) with sticky lock behavior, establishes the dependabot-core integration contract with golden-captured JSON bodies, and fixes several correctness issues in
--no-onboardand--no-fixpaths.Why
Three motivations converged on this branch:
Tag narrowing:
actions/checkout@v4should resolve tov4.2.0(the most precise semver tag pointing at the same commit). This reduces blast radius —v4can move to any4.x, butv4.2.0is immutable. Users who explicitly chose a splat ref keep it (sticky precision).Dependabot contract: dependabot-core consumes
check --jsonoutput to enforce lockfile pins during dependency updates. The CLI's exit codes, finding categories, severity levels, and JSON shape ARE the integration contract. Without a shared test corpus, the two sides drift silently.Correctness fixes:
--no-fixwasn't showing the SSO authorization URL.--no-onboardon fresh repos exited 1 (blocking dependabot runs that can't bootstrap). Transient 403s during re-resolution silently dropped existing pins.How
Tag narrowing (
--no-narrowflag)v4→v4.2.0) to most precise semver tag at the same commit--no-narrow: skip narrowing entirelyv4(imprecise), re-pin keepsv4— respects the user's prior choiceDependabot contract (
dependabotscenario category)catalog.ymlcovering the exact--jsonbody shape dependabot-core parsesruby test/integration/run.rb --golden-update dependabotruns each scenario through the real binary and writes the literal JSON stdout intoexpect.golden_jsonExit code semantics (
--no-onboard)OnboardingRequiredfindings downgraded toseverity: infoandIsValid() = truefindings[]at exit 0 for consumer observabilityPin retention on transient failures
retainImpostorPins→retainUnresolvablePins: broadened to also retain entries where resolution status isUnresolved(not just impostor commits)Bug fixes
--no-fixmode--no-onboardsummary names each refused workflow/action (was just a count)Risk
--no-onboardon fresh repos — this is the dependabot consumer, who requested the changecatalog.yml— they're already tracking this branchTesting
go test ./...— 16 packages)retainUnresolvablePinshas dedicated unit tests for impostor, no-op, and unresolved casesCommit guide
feat: add --no-narrow flag and respect locked ref precision— Core narrowing implementationnarrowing: nudge non-semver refs, global sticky precision— UX polishrename mutable-ref to version-ref— Terminology alignmentport contract-essential pieces from PR #36— Bring forward shared catalog infrascenarios: add narrowing, onboarding, and lockfile recovery cases— Test matrix expansioncheck: surface SSO authorization URL in --no-fix mode— Bug fixpin: retain existing pins when re-resolution fails— 403/transient fixonboard: name refused workflows in --no-onboard summary— UX improvementscenarios: add dbot_transient_403_drops_pin contract scenario— Contract coverageonboard: exit 0 when only onboarding-required findings remain— Exit code semanticsscenarios: add dbot_impostor_blocks and dbot_forgery_blocks stubs— Stub placeholdersharness: golden capture mode for dependabot contract scenarios— Golden capture + IsValid fixFollow-ups
dependabotlockfile_templatefield in scenarios for self-contained vendoring