Pinning: repair SHA refs from existing lock metadata - #116
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Rekeying a composite dependency can discard its transitive dependency graph.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
internal/pin/plan.go — Rekeying only this entry drops the repaired action's transitive dependency graph. On the fast path,… |
What changed in this PR
Repairs SHA-based references using symbolic metadata already stored in the lockfile.
Changes:
- Preserves tag and branch metadata in verified entries.
- Rewrites direct SHA references without API requests.
- Adds an end-to-end regression test.
| File | Description |
|---|---|
internal/pin/plan.go |
Restores symbolic refs from lock metadata. |
cmd/gh-actions-lock/selfrepository_test.go |
Tests nested self-repository action repair. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Partial scans can rewrite a shared local action without updating every affected workflow’s lock entry.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
internal/pin/plan.go — When pinning only a subset of workflows, this rewrite can modify a $/ action file that is also… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
internal/pin/plan.go — Rekeying only this entry drops the repaired action's transitive dependency graph. On the fast path,… View resolved comment |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Scan-scope detection and dependency-edge preservation can currently reject valid runs or lose transitive lockfile entries.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
|
internal/pin/plan.go — The partial-scan guard currently rejects the command even when --no-narrow is set (or no tagger… |
|
internal/pin/plan.go — Inferring scan scope from lockfile keys makes a real full-directory scan look partial whenever the… |
|
internal/lockfile/state.go — The SHA-keyed entry is consulted only when the destination symbolic key is absent. If both keys… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
internal/pin/plan.go — When pinning only a subset of workflows, this rewrite can modify a $/ action file that is also… View resolved comment |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Partial-scan detection can reject valid full scans and unnecessarily fails when narrowing is disabled.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
|
internal/pin/plan.go — This guard also rejects partial scans with --no-narrow, even though narrowVerifiedEntries… |
 |
internal/pipeline/checks/parsed.go — SelfActionRefs is exported but has no doc comment describing how it differs from Refs and… |
|
internal/pipeline/checks/finding.go — SelfActionRefs is exported but undocumented; its local-action-only scope is important to the… |
Issues resolved since last review (3)
| Severity | Finding |
|---|---|
|
internal/lockfile/state.go — The SHA-keyed entry is consulted only when the destination symbolic key is absent. If both keys… View resolved comment |
|
internal/pin/plan.go — Inferring scan scope from lockfile keys makes a real full-directory scan look partial whenever the… View resolved comment |
|
internal/pin/plan.go — The partial-scan guard currently rejects the command even when --no-narrow is set (or no tagger… View resolved comment |
Suppressed comments (1)
internal/pin/plan.go:91
- Comparing the report only with current lockfile keys misclassifies a full scan whenever the lockfile contains a deleted workflow. The CLI intentionally discovers those stale keys and prunes them only after
Plan, so a normal full run that should prune a deleted workflow will instead hit the partial-scan error when repairing a local action. Pass the invocation's actual full/partial-scan status into planning, or exclude the already-detected stale keys before this check.
poolErr := pinpool.RunTyped(opts.Pool, ctx, "Planning pins",
items,
func(iwr indexedWR) string { return "planning " + iwr.wr.Path },
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The global sticky-ref guard can prevent the intended SHA repair, and branch repairs produce inaccurate summary output.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
|
internal/pin/plan.go — Handle the bare-SHA metadata repair before the sticky imprecise-ref guard. prevImpreciseNWO is… |
|
internal/pin/plan.go — A branch-only metadata repair also sets AutoFixedRef, so Record.Narrowed() passes this entry to… |
Issues resolved since last review (3)
| Severity | Finding |
|---|---|
|
internal/pipeline/checks/finding.go — SelfActionRefs is exported but undocumented; its local-action-only scope is important to the… View resolved comment |
|
internal/pipeline/checks/parsed.go — SelfActionRefs is exported but has no doc comment describing how it differs from Refs and… View resolved comment |
|
internal/pin/plan.go — This guard also rejects partial scans with --no-narrow, even though narrowVerifiedEntries… View resolved comment |
| if opts.prevImpreciseNWO[strings.ToLower(e.NWO)] { | ||
| continue | ||
| } | ||
| if parserlock.IsFullSha(e.Ref) { |
| if newRef == "" { | ||
| newRef = e.OnBranch | ||
| } |
What
Repair bare-SHA references in
$/action files when an existing lockfile already records the matching symbolic tag or branch.Fixes #114.
Why
Fresh onboarding with
v0.1.7-rc.1rewrites the reported SHA tov3.2.0, but lockfiles generated byv0.1.6can retain the SHA as the dependency key while storingv3.2.0in its metadata. The verified-entry path discarded that metadata, so upgrading left the action source at the SHA and regressed the lockfile ref back to it.How
Carry existing tag and branch metadata into verified entries. When a direct verified entry is keyed by a full SHA, reuse that metadata to rewrite the source and rekey the lockfile without another API request.
Testing
$/action and av0.1.6-shaped lockfilego test ./...