{{ message }}
Repository navigation
Make local-action migration default-on with a --no-migrate-local-actions opt-out - #92
Merged
nodeselector merged 2 commits intoJul 24, 2026
Merged
Conversation
Local-action migration (same-repo `./…` composite refs → inherently-pinned `$/…`) was gated behind opt-in `--migrate-local-actions`. Flip it to run by default on any fix run, and add a real named opt-out that mirrors the existing `--no-fix` negative-flag convention. - Gate at both entrypoints (run.go, verify.go) becomes `!opts.noMigrateLocalActions && !opts.noFix`; the `!opts.noFix` guard is preserved exactly so read-only runs never touch disk. - Replace the opt-in flag with `--no-migrate-local-actions` (`opts.noMigrateLocalActions`, default false). The old flag was unreleased, so no back-compat no-op alias is kept. - Update diagnose.go remediation strings and the README to say migration is automatic and can be disabled with the opt-out flag. - Update tests/catalog: drop the now-default flag, add coverage for the default-on rewrite, the `--no-fix` no-touch guarantee, and the opt-out; add an opt-out catalog scenario. An invalid `$/…` ref now aborts during the default migration pass, so the affected command test asserts that error. Copilot-Session: c95a173f-d4b8-43a9-9a78-880ec48023d6
There was a problem hiding this comment.
Pull request overview
Makes local-action migration default-on for fix runs, with an explicit opt-out.
Changes:
- Replaces the opt-in flag with
--no-migrate-local-actions. - Updates diagnostics and documentation.
- Adds command and integration coverage for default and opt-out behavior.
Show a summary per file
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
cmd/gh-actions-lock/migrate_test.go:276
- This drops the command error, so the opt-out test also passes if the new flag is rejected (or execution fails before reaching migration), because the workflow remains unchanged either way. Require the expected successful command execution before asserting the file contents.
_, _, _ = runCommandWithHTTP(t, transport, "--no-migrate-local-actions", wf)
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Medium
| t.Run("--no-fix leaves file untouched", func(t *testing.T) { | ||
| transport := &requestCountingTransport{} | ||
| wf := setup(t) | ||
| _, _, _ = runCommandWithHTTP(t, transport, "--no-fix", wf) |
Comment on lines
+531
to
+533
Default-on migration front-runs diagnosis, so the three self_repository_* scenarios that feed intentionally-invalid `$/` refs were aborting in the migration pass instead of reaching --verify-local diagnosis. Their purpose is to prove the real binary diagnoses bad `$/` refs, and opting out of migration is now how you reach that path, so add --no-migrate-local-actions to each. The abort behavior itself stays covered by the Go command tests. Also acts on review feedback: - Remove the migrate_local_actions_opt_out catalog scenario. It duplicated the deterministic TestMigrateLocalActions_DefaultOnAndOptOut Go test with no real-binary delta; the opt-out now gets compiled-binary coverage through the self_repository_* scenarios where it has a genuine integration purpose. - Assert the command result in the --no-fix and --no-migrate-local-actions subtests so they prove the guard rather than merely the absence of a write. Copilot-Session: c95a173f-d4b8-43a9-9a78-880ec48023d6
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.
Why
Rewriting same-repo
./…composite action refs to the inherently-pinned$/…form was gated behind the opt-in--migrate-local-actionsflag, so most fix runs left non-compliant local refs in place. Since the rewrite is safe (only paths that resolve to an in-repo action file are migrated) and is what we want in the common case, this makes it the default and gives users a clean way to opt out.What changed
run.goandverify.go) becomes!opts.noMigrateLocalActions && !opts.noFix. Migration now runs automatically on any fix run. The!opts.noFixguard is preserved exactly, so read-only runs still never touch disk.--no-migrate-local-actions(opts.noMigrateLocalActions, default false), mirroring the repo's existing--no-fixnegative-flag convention rather than relying on--flag=false. The old--migrate-local-actionsflag was unreleased, so no back-compat no-op alias is kept.diagnose.goremediation strings and the README to say migration is automatic and can be disabled with the opt-out flag.The rewrite algorithm itself (
MigrateLocalActionsToSelfRepository,migrateLocalActions) is untouched.Heads-up for review
One behavior change worth a conscious look: because migration now runs by default, an invalid
$/…ref is validated during the migration pass and aborts the whole run early, where a plain run previously proceeded to diagnosis. The affected pre-existing command test was updated to assert that error; the invariants it guards (no network, file left untouched) still hold. If we'd rather soften this (downgrade a migration-validation failure to a warning and skip that file instead of aborting), that can be a follow-up.Tests
TestMigrateLocalActions_DefaultOnAndOptOutcovers: (a) a plain fix run rewrites./.github/actions/footo$/…, (b)--no-fixleaves the file untouched, (c)--no-migrate-local-actionssuppresses the rewrite.migrate_local_actions_opt_outscenario. Both are[stub]-tagged so they run in blocking CI.Verified with gofmt,
go build ./...,go vet,go test ./..., plus a manual smoke on a throwaway repo confirming all three behaviors.Out of scope
Job-level local reusable-workflow calls (
uses: ./.github/workflows/x.yml) remain excluded. The rewrite algorithm is unchanged.