Skip to content

Local path action handling, narrowing expansion, and rerun perf fix - #50

Merged
nodeselector merged 21 commits into
mainfrom
nodeselector/skip-local-path-workflows
Jun 15, 2026
Merged

Local path action handling, narrowing expansion, and rerun perf fix#50
nodeselector merged 21 commits into
mainfrom
nodeselector/skip-local-path-workflows

Conversation

@nodeselector

Copy link
Copy Markdown
Collaborator

Why

Repos like github/mcv3-boot use local path actions (uses: ./...) in many workflows, have actions pinned with bare SHAs alongside tag refs, and include same-owner private repos that publish semver releases. The pin tool didn't handle any of these cases well: local-path workflows were silently onboarded (unsupported), non-semver refs like @main on public actions were left unnarrowed, same-owner private repos with semver tags were skipped entirely, and reruns were needlessly slow because local-path workflows triggered redundant network calls.

What changed

Local path action gate -- Workflows with uses: ./... steps are now excluded from lockfile onboarding. If a workflow is already onboarded and then adds local path actions, it surfaces as an error with remediation guidance. Wired through the finding/category/formatter stack with two test scenarios.

Narrowing expansion -- Three related changes broaden tag narrowing coverage:

  • Non-semver refs (like @main) on public actions are now narrowed to full semver tags
  • When no exact tag matches a SHA, BestAncestorTag walks back to the latest ancestor semver release (handles repos with release-branch publishing like codeql-action)
  • Same-owner private repos that publish semver tags are no longer skipped by the isInternal guard

Verified dep narrowing -- Already-recorded deps that have imprecise refs (branch names, partial semver) are now narrowed at all three return paths in planWorkflow. A narrowedNWOs set prevents ReverseLookup from overwriting narrowed refs back to branch names.

Reachability perf -- checkReachabilityOnce now stashes its discovered branch in branchHintBySHA so DiscoverContaining picks it up in Phase 0 and skips the expensive full branch scan. For codeql-action (~250 branches), this cut pin.Plan from 1m49s to 5.9s.

Rerun perf -- Local-path workflows were being fed through the resolve/reachability pipeline even though diagnose skips them. Marking them Resolved=true early eliminates all network calls on steady-state reruns. PartitionRefs also now matches bare-SHA workflow refs against lockfile deps by SHA, not just by tag ref. mcv3-boot rerun: ~6s to ~200ms, 65 to 0 HTTP requests.

Test harness -- The shell REPL now supports a full adhoc lifecycle: run owner/repo keeps the context alive, rerun re-executes against the same clone (with delta-only diffs via git checkpointing), rerun --rescan / rescan passes --rescan through, edit opens the scenario dir in $EDITOR, and done tears down.

Validation

  • All unit tests pass
  • Live validation against github/mcv3-boot (78 workflows, 22 local-path) and github/launch
  • Profile-verified: steady-state rerun is 200ms / 0 HTTP requests

Workflows that use local path actions (uses: ./some-path) are now
bailed out of entirely — no lockfile entry is created for the
workflow, even if it also has remote action refs. The diagnose
phase emits a LocalAction warning and returns early.

Previously local paths were silently discarded and only the remote
refs were onboarded, which is wrong: we don't support local actions
yet, so the lockfile would be incomplete and misleading.
If a workflow already has a lockfile entry and then adds a local
path action (uses: ./…), the check now fails with an error instead
of silently skipping. The user must remove the local path steps or
split them into a separate workflow.

New workflows with local paths still get the non-blocking warning
and are skipped from onboarding.
Covers the case where a previously-onboarded workflow adds a local
path action (uses: ./path). This should produce a hard error (exit 1)
since we can't track local paths in the lockfile.
The interactive shell REPL was defined after a private keyword,
making it inaccessible from the top-level runner invocation.
Previously only partial semver refs (v4, v3.1) were narrowed to
patch tags. Non-semver refs like @main were left as-is, even on
third-party public actions where tracking a branch is fragile.

Now any non-full-semver ref on a public action gets narrowed to the
best patch tag for its resolved SHA when one exists. Same-owner
internal/private repos are still skipped (they may intentionally
track a branch).
When BestPatchTagForSHA finds no tag at the exact SHA (common for
repos where dependabot walks the commit forward past the latest
release), fall back to BestAncestorTag — checks the latest 3 semver
tags and returns the first one that's an ancestor of the current SHA.

Covers both bare-SHA refs and non-semver refs like @main. For
toshimaru/auto-author-assign@main this narrows to v3.0.3 instead
of leaving a 'pinned without full semver tag' warning.
Verified deps (already in the lockfile) with imprecise refs like @main
or bare SHAs were never reaching the narrowing block — they took the
fast inventory path and skipped resolve/narrow entirely.

Add narrowVerifiedEntries() to upgrade imprecise refs on verified entries
at all three return points in planWorkflow. Also track which NWOs were
narrowed and restore their refs after ReverseLookup, which otherwise
overwrites the narrowed semver tag with the branch name (e.g. main).
CheckReachability and DiscoverContaining both do the same 3-phase
branch scan independently. For repos like codeql-action with ~250
branches, the redundant Phase 2 scan in DiscoverContaining added ~250
sequential CompareCommits API calls and ~1m30s of wall time.

Store the discovered branch in branchHintBySHA so DiscoverContaining's
Phase 0 (named branches) picks it up immediately, skipping the
expensive Phase 1+2 scans entirely. github/launch drops from ~2m10s
to ~30s.
The isInternal skip was too aggressive — it blocked narrowing for all
same-owner private repos, even ones that publish semver releases like
github/go-linter. The tag lookup already no-ops gracefully for repos
without tags, so the guard was unnecessary.
`run owner/repo` now keeps the cloned repo and lockfile state so
`rerun` re-executes against the same checkout. This makes it easy to
verify idempotent re-pin behavior (fresh pin → rerun should be a
fast noop). The rerun output now includes timing, diff, and profile
info matching the first run's format.
After the first `run`, git-commit the working tree so the rerun diff
shows only what changed since the last execution. When nothing changed
(the expected idempotent case), prints '✓ no changes from previous run'
instead of repeating the full lockfile diff. Each rerun also checkpoints
so chained reruns stay clean.
`rerun --rescan` appends --rescan to the binary invocation so the
CLI rescans workflows against the existing lockfile. Removed the
standalone rescan harness command.
Spawns a real bash subshell so vim/nvim gets a proper TTY context
with the scenario dir as cwd.
Local-path workflows are bailed out at diagnose time, but their action
refs were still fed through the resolve and reachability phases because
PartitionRefs found no matching lockfile entries. On mcv3-boot this
caused 65 redundant HTTP calls (~6s) on every rerun.

Fix: mark local-path workflows as Resolved=true before the partition
loop so they never enter the network path. Also teach PartitionRefs to
match bare-SHA refs against lockfile deps by SHA (not just by tag ref)
for correctness in mixed-pinning repos.

Result: mcv3-boot rerun drops from ~6s to ~200ms, 0 HTTP requests.
Copilot AI review requested due to automatic review settings June 15, 2026 17:58
GitHub Advanced Security started work on behalf of nodeselector June 15, 2026 17:59 View session
GitHub Advanced Security finished work on behalf of nodeselector June 15, 2026 18:02

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.

⚠️ Not ready to approve

There are user-visible correctness and reporting issues (AutoFixedRef misuse during verified narrowing, LocalAction error being treated as a warning too, and JSON output omitting LocalAction errors) plus a command-injection risk in the new edit command.

Pull request overview

This PR improves gh-actions-lock handling of workflows that use local-path actions, expands ref/tag narrowing so more action refs converge on stable full semver tags, and reduces rerun/reachability overhead by avoiding unnecessary network work (plus upgrades the integration shell workflow for faster iteration).

Changes:

  • Add local-path action detection and reporting (warning for not-yet-onboarded workflows; hard error when an onboarded workflow introduces local actions), and skip network resolution for these workflows on steady-state runs.
  • Expand narrowing to cover non-semver refs and add an “ancestor semver tag” fallback when no exact tag points at a SHA; also narrow already-recorded (verified) deps when possible and prevent ReverseLookup from undoing narrowing.
  • Performance/UX improvements: stash reachability-discovered branch hints; improve rerun behavior; enhance integration shell with rerun/rescan/edit/done.
File summaries
File Description
test/scenarios/catalog.yml Updates/extends scenario coverage for local-path action behavior.
test/integration/harness.rb Adds interactive shell lifecycle commands and rerun/rescan UX improvements.
internal/tag/tagging.go Introduces BestAncestorTag to support ancestor-based semver narrowing.
internal/resolve/reachability.go Caches discovered branch hints for faster subsequent containing-branch discovery.
internal/pipeline/run.go Skips resolution work for local-path workflows on non-rescan runs.
internal/pipeline/run_test.go Adds test coverage for SHA-based matching in PartitionRefs.
internal/pipeline/parse.go Plumbs local-path action extraction into parsed workflow state.
internal/pipeline/diagnose.go Emits LocalAction findings (warn vs error based on onboarding state).
internal/pipeline/diagnose_test.go New tests validating local-path finding severity behavior.
internal/pipeline/checks/parsed.go Enhances “recorded vs unrecorded” partitioning to match by SHA as well as ref.
internal/pipeline/checks/finding.go Treats LocalAction as non-attention category and warning-classified (needs tweak for error case).
internal/pipeline/checks/category.go Adds local-action category.
internal/pipeline/checks/category_test.go Freezes category string and updates inconclusive tests.
internal/pin/plan.go Adds narrowing for verified entries, ancestor narrowing fallback, and preserves narrowing across ReverseLookup.
cmd/gh-actions-lock/format/terminal.go Renders LocalAction and adds warning bucket for skipped workflows.
cmd/gh-actions-lock/format/json.go Currently filters out LocalAction findings from JSON output (needs tweak for error case).

Copilot's findings

  • Files reviewed: 16/16 changed files
  • Comments generated: 5

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.

Comment thread internal/pin/plan.go Outdated
Comment thread internal/pipeline/checks/finding.go Outdated
Comment thread cmd/gh-actions-lock/format/json.go Outdated
Comment thread cmd/gh-actions-lock/format/json.go Outdated
Comment thread test/integration/harness.rb Outdated
…orkflows

New scenarios:
- fresh_branch_ref_narrows: @main on public action narrows to full semver
- fresh_branch_ref_no_narrow: --no-narrow keeps @main as-is
- onboarded_branch_ref_narrows: verified dep at @main gets narrowed on repin
- local_action_only: workflow with only local path steps, no remote refs

Replaces the now-wrong fresh_branch_ref_skipped which asserted @main
stayed as main (before non-semver narrowing was added).
GitHub Advanced Security started work on behalf of nodeselector June 15, 2026 18:12 View session
- narrowVerifiedEntries: set AutoFixedRef to just the ref, not NWO@ref
- IsWarning: return false for error-level LocalAction findings
- json: only skip warning-level LocalAction findings, not errors
- harness edit: use Shellwords.split + system(*cmd, chdir:) instead
  of interpolating EDITOR into a bash -c string
GitHub Advanced Security finished work on behalf of nodeselector June 15, 2026 18:14
GitHub Advanced Security started work on behalf of nodeselector June 15, 2026 18:14 View session
GitHub Advanced Security finished work on behalf of nodeselector June 15, 2026 18:15
@nodeselector
nodeselector merged commit a79ce69 into main Jun 15, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants