Skip to content

fix(workflows): pin scripts checkout to job.workflow_sha for version lock#14

Merged
lml2468 merged 1 commit into
mainfrom
fix/scripts-ref-version-lock
May 15, 2026
Merged

fix(workflows): pin scripts checkout to job.workflow_sha for version lock#14
lml2468 merged 1 commit into
mainfrom
fix/scripts-ref-version-lock

Conversation

@lml2468
Copy link
Copy Markdown
Contributor

@lml2468 lml2468 commented May 15, 2026

Summary

Three reusable notify workflows previously checked out scripts with ref: main. This caused a version drift issue: when callers reference @v1, the workflow YAML is frozen at v1 but the Python scripts were still pulled from main.

Change

Replace ref: main with ref: ${{ job.workflow_sha }} in the checkout step of:

  • .github/workflows/octo-ci-status.yml
  • .github/workflows/octo-issue-feed.yml
  • .github/workflows/octo-pr-feed.yml

job.workflow_sha is the commit SHA of the called reusable workflow file itself, ensuring scripts are always checked out at the exact same commit as the workflow — works correctly with both branch refs and version tags.

Why This Matters

This is a prerequisite for tagging v1. Without this fix, @v1 callers would always run the latest main scripts regardless of which workflow version they pinned to.

Review Notes

  • All other aspects of the workflows pass review (GITHUB_TOKEN availability, permissions, trigger design, Python syntax)
  • Python scripts compile OK, hardcoded group_ids are correct by design
  • After this PR merges, we can safely tag v1 on main and update all callers to reference @v1

…lock

Three reusable notify workflows (octo-ci-status, octo-issue-feed, octo-pr-feed)
previously checked out scripts with ref: main. This caused a version drift
issue: when callers reference @v1, the workflow YAML is frozen at v1 but the
Python scripts were still pulled from main.

Replace ref: main with ref: ${{ job.workflow_sha }}, which is the commit SHA
of the called workflow file. This guarantees scripts and workflow are always
at the exact same commit, working correctly with both branch refs and tags.

Prerequisite for tagging v1.
Copy link
Copy Markdown

@yujiawei yujiawei left a comment

Choose a reason for hiding this comment

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

Code Review — PR #14 (.github)

Summary

This change replaces ref: main with ref: ${{ job.workflow_sha }} in the actions/checkout step of three reusable workflows (octo-ci-status.yml, octo-issue-feed.yml, octo-pr-feed.yml). The motivation — eliminating the version-drift between the workflow YAML (frozen at the caller's tag) and the Python scripts (previously always pulled from main) — is valid and well explained. The implementation is correct.

Verdict: APPROVED.

1. Correctness of job.workflow_sha

Verified against GitHub Actions contexts documentation:

job.workflow_sha — The commit SHA of the workflow file that defines the current job.

For a job in a reusable workflow, this resolves to the SHA of the called (reusable) workflow file, not the caller's entry-point. That is exactly the value needed here:

  • Caller pins Mininglamp-OSS/.github/.github/workflows/octo-ci-status.yml@v1 → at job start GitHub resolves @v1 to a commit SHA → job.workflow_sha is that SHA → the actions/checkout step pulls scripts/ at that same SHA. Workflow YAML and Python scripts are now guaranteed to be from the same commit.
  • For callers still using @main, behavior remains effectively equivalent: job.workflow_sha is the current tip of main, and scripts are checked out at the same SHA the workflow ran from. This even gains a small consistency benefit — if main advances mid-run, the script checkout no longer races ahead of the workflow that started.

2. Verification — file-by-file

All three changes are identical in shape (12 additions / 12 deletions across 3 files, confirmed via gh pr diff):

  • .github/workflows/octo-ci-status.yml:45-56 — checkout step now uses job.workflow_sha. Other settings (persist-credentials: false, sparse-checkout: scripts, pinned-SHA checkout action 11bd71901bbe5b1630ceea73d27597364c9af683 v4.2.2) are preserved. ✅
  • .github/workflows/octo-issue-feed.yml:44-55 — same change, same surrounding context preserved. ✅
  • .github/workflows/octo-pr-feed.yml:53-64 — same change, same surrounding context preserved. ✅

The replacement comment is accurate and helpful for future maintainers.

3. Completeness

Searched the entire repo for any remaining ref: main / ref: master patterns in workflow files:

$ rg 'ref:\s*(main|master)' .github/
(no matches)

Other workflows in the repo (auto-add-to-project.yml, issue-welcome.yml) do not perform an actions/checkout of Mininglamp-OSS/.github, so they do not need this change. The fix is complete.

4. Minor observations (non-blocking)

a) job.workflow_sha is not available on GitHub Enterprise Server

The same docs note: "(not available on GitHub Enterprise Server)". On GHES the context expression resolves to an empty string, which would make actions/checkout fail (or pull the default branch, depending on action version). Since Mininglamp-OSS lives on GitHub.com and these reusable workflows are intended for that org's repos, this is a non-issue today. If these workflows are ever reused from a GHES environment, this would need a fallback. Consider a one-line note in the README or in the workflow comment, e.g.:

# NOTE: requires GitHub.com (job.workflow_sha is not available on GitHub Enterprise Server).

Optional — not blocking the merge.

b) The PR body is the prerequisite for tagging v1

The PR description states this is a prerequisite for tagging v1. After merge, the recommended sequence is:

  1. Merge this PR to main.
  2. Tag v1 at the resulting commit.
  3. Update caller workflows from @main to @v1.

If v1 is tagged at any commit before this fix, the version-drift problem would persist for that tagged version. This is a release-process concern rather than a code concern — flagging for awareness only.

c) Behavior across forks / private callers

job.workflow_sha is the SHA of the called workflow's file, which lives in Mininglamp-OSS/.github. Since the repo is public, anonymous actions/checkout (with persist-credentials: false) succeeds without a token. No change in behavior here. ✅

5. Risk assessment

  • Backward compatibility: caller workflows do not change — drop-in replacement. ✅
  • Security: no expansion of permissions, action still pinned by SHA, credentials still not persisted, sparse-checkout still scoped to scripts/. ✅
  • Failure modes: the only realistic failure is GHES (per 4a) or if job.workflow_sha ever returns an empty string for an unexpected reason. Worst case the checkout fails fast in the notify job — it does not silently run stale code. ✅

Recommendation

Approve and merge. Optionally append a "requires GitHub.com" note to the comment block, but this is not blocking.

Copy link
Copy Markdown

@Jerry-Xin Jerry-Xin left a comment

Choose a reason for hiding this comment

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

APPROVED ✅

Mechanical but important fix. All 3 reusable notify workflows (octo-ci-status, octo-issue-feed, octo-pr-feed) now pin their script checkout to job.workflow_sha instead of main, fixing the version drift where tagged callers (@v1) would get frozen workflow YAML but bleeding-edge scripts.

Verified:

  • job.workflow_sha is the correct context for workflow_call reusable workflows — resolves to the caller's pinned ref, not github.sha (which would be the caller repo's commit)
  • All 3 files consistently updated, no remaining ref: main in any workflow
  • The other 2 workflows (auto-add-to-project, issue-welcome) don't check out scripts, so no change needed
  • actions/checkout SHA pin unchanged (v4.2.2)
  • Comments updated to explain the new behavior

CI note: add-to-project failure is the same pre-existing infra token issue (see PR #13), unrelated.

Clean prerequisite for the v1 tag. Ship it.

@lml2468 lml2468 merged commit 1aab7b3 into main May 15, 2026
1 of 2 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.

3 participants