[PR #69] [MERGED] fix(groom): default workflows_ref to github.job_workflow_sha so assets can't desync from the uses: pin (BE-4169) #80

New issue

Closed

opened 2026-10-26 22:36:01 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:36:01 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/69
Author: @mattmillerai
Created: 7/24/2026
Status: ✅ Merged
Merged: 8/4/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-4169-groom-workflows-ref-default

📝 Commits (3)

55385c6 fix(groom): default workflows_ref to github.job_workflow_sha so assets can't desync from the uses: pin (BE-4169)
e662e56 Merge remote-tracking branch 'origin/main' into matt/be-4169-groom-workflows-ref-default
31bc5a1 Merge remote-tracking branch 'origin/main' into matt/be-4169-groom-workflows-ref-default

📊 Changes

1 file changed (+41 additions, -13 deletions)

View changed files

📝 .github/workflows/groom.yml (+41 -13)

📄 Description

ELI-5

The groom workflow reads its instruction files (the finder/verifier/builder briefs + the dedup ledger) out of this repo. Until now, callers had to hand-copy the same commit SHA into two places — the uses: line and a workflows_ref: input — to make the code and the instructions match. Nobody enforced that they stayed equal, so a caller that forgot workflows_ref ran the pinned code but read instructions from moving main, and a pin bump that touched only one line ran code and instructions at different commits. This change makes the workflow default to pulling its assets from the exact commit it was itself resolved from (github.job_workflow_sha), so callers don't have to mirror anything and the two can't drift.

What changed

.github/workflows/groom.yml only:

Changed the effective checkout ref of every _groom_assets step from ${{ inputs.workflows_ref }} to ${{ inputs.workflows_ref || github.job_workflow_sha }} (all six asset checkouts: finder briefs, verifier briefs, both ledger checkouts, the builder brief, and the signature marker — leaving any one bare would reintroduce the desync).
Changed the workflows_ref input default from main to '' (empty), so an unset input falls through to github.job_workflow_sha — documented as "for jobs using a reusable workflow, the commit SHA for the reusable workflow file."
Kept workflows_ref as an explicit override (e.g. testing briefs from a branch) and rewrote its description to say so.
Dropped the workflows_ref: <sha> line from the caller-pattern comment block and refreshed the adjacent security comment.

Why this is safe

groom.yml is on: workflow_call only, so github.job_workflow_sha is always populated with the reusable workflow's own resolved commit when its jobs run. The || returns the first truthy operand: an explicit workflows_ref still wins; an empty/unset one falls back to the auto-pin. The degenerate "both empty" case cannot occur for a workflow_call-only reusable, and even if it did, an empty ref just checks out github-workflows' default branch — no worse than the old default: main. default: main was the naive initial default introduced with the workflow itself in #49 (BE-3872); this ticket is its intended correction (deferred from the review thread on comfy-cloud-mcp-server#786).

Scope / follow-ups (deliberately not in this PR)

Both remaining caller cleanups must wait until each caller's uses: pin is bumped to a SHA that includes this fix — dropping workflows_ref from a caller whose uses: still points at the pre-fix groom.yml would make that old workflow default to main, i.e. the exact drift bug:

This repo's own ci-groom.yml still passes workflows_ref: 07154fb… (its uses: pins the old groom.yml). Left as-is; it becomes removable once the pin is bumped (bump-callers machinery).
comfy-cloud-mcp-server pilot caller (its lines 49–51, per the ticket) — a follow-up in that repo after its pin is bumped.

Verification

python3 -m unittest discover -s .github/groom/tests → 48 tests pass.
YAML parses; all six bare inputs.workflows_ref refs confirmed replaced.
Human pre-merge check requested by the ticket: confirm github.job_workflow_sha is populated on a SHA-pinned uses: dispatch (a live dry-run of ci-groom.yml against a SHA carrying this change). Not runnable from the coding loop (needs a live run + secrets).
Note: actionlint 1.7.12 flags github.job_workflow_sha as an unknown context property — that's a stale-schema false positive (the property is documented and quoted above); this repo's CI does not run actionlint on workflows.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/69 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/24/2026 **Status:** ✅ Merged **Merged:** 8/4/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-4169-groom-workflows-ref-default` --- ### 📝 Commits (3) - [`55385c6`](https://github.com/Comfy-Org/github-workflows/commit/55385c639d8d475c6f44177b64b68bd9fd24c4b3) fix(groom): default workflows_ref to github.job_workflow_sha so assets can't desync from the uses: pin (BE-4169) - [`e662e56`](https://github.com/Comfy-Org/github-workflows/commit/e662e568df2c37f0ab059483b4a85d98766c05b4) Merge remote-tracking branch 'origin/main' into matt/be-4169-groom-workflows-ref-default - [`31bc5a1`](https://github.com/Comfy-Org/github-workflows/commit/31bc5a1b42c5199f6fa78cbaa8dd05bf627c9734) Merge remote-tracking branch 'origin/main' into matt/be-4169-groom-workflows-ref-default ### 📊 Changes **1 file changed** (+41 additions, -13 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/groom.yml` (+41 -13) </details> ### 📄 Description ## ELI-5 The groom workflow reads its instruction files (the finder/verifier/builder briefs + the dedup ledger) out of *this* repo. Until now, callers had to hand-copy the same commit SHA into two places — the `uses:` line *and* a `workflows_ref:` input — to make the code and the instructions match. Nobody enforced that they stayed equal, so a caller that forgot `workflows_ref` ran the pinned code but read instructions from moving `main`, and a pin bump that touched only one line ran code and instructions at different commits. This change makes the workflow default to pulling its assets from the *exact commit it was itself resolved from* (`github.job_workflow_sha`), so callers don't have to mirror anything and the two can't drift. ## What changed `.github/workflows/groom.yml` only: - Changed the effective checkout ref of every `_groom_assets` step from `${{ inputs.workflows_ref }}` to `${{ inputs.workflows_ref || github.job_workflow_sha }}` (all **six** asset checkouts: finder briefs, verifier briefs, both ledger checkouts, the builder brief, and the signature marker — leaving any one bare would reintroduce the desync). - Changed the `workflows_ref` input `default` from `main` to `''` (empty), so an unset input falls through to `github.job_workflow_sha` — [documented](https://docs.github.com/en/actions/reference/contexts-reference#github-context) as "for jobs using a reusable workflow, the commit SHA for the reusable workflow file." - Kept `workflows_ref` as an explicit override (e.g. testing briefs from a branch) and rewrote its description to say so. - Dropped the `workflows_ref: <sha>` line from the caller-pattern comment block and refreshed the adjacent security comment. ## Why this is safe `groom.yml` is `on: workflow_call` only, so `github.job_workflow_sha` is always populated with the reusable workflow's own resolved commit when its jobs run. The `||` returns the first truthy operand: an explicit `workflows_ref` still wins; an empty/unset one falls back to the auto-pin. The degenerate "both empty" case cannot occur for a `workflow_call`-only reusable, and even if it did, an empty ref just checks out github-workflows' default branch — no worse than the old `default: main`. `default: main` was the naive initial default introduced with the workflow itself in #49 (BE-3872); this ticket is its intended correction (deferred from the review thread on comfy-cloud-mcp-server#786). ## Scope / follow-ups (deliberately not in this PR) Both remaining caller cleanups must wait until each caller's `uses:` pin is bumped to a SHA that includes *this* fix — dropping `workflows_ref` from a caller whose `uses:` still points at the pre-fix `groom.yml` would make that old workflow default to `main`, i.e. the exact drift bug: - **This repo's own `ci-groom.yml`** still passes `workflows_ref: 07154fb…` (its `uses:` pins the old groom.yml). Left as-is; it becomes removable once the pin is bumped (bump-callers machinery). - **comfy-cloud-mcp-server** pilot caller (its lines 49–51, per the ticket) — a follow-up in that repo after its pin is bumped. ## Verification - `python3 -m unittest discover -s .github/groom/tests` → 48 tests pass. - YAML parses; all six bare `inputs.workflows_ref` refs confirmed replaced. - **Human pre-merge check requested by the ticket:** confirm `github.job_workflow_sha` is populated on a SHA-pinned `uses:` dispatch (a live dry-run of `ci-groom.yml` against a SHA carrying this change). Not runnable from the coding loop (needs a live run + secrets). - Note: `actionlint` 1.7.12 flags `github.job_workflow_sha` as an unknown context property — that's a stale-schema false positive (the property is documented and quoted above); this repo's CI does not run actionlint on workflows. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:36:01 +00:00