[PR #27] [MERGED] ci(bump-callers): move caller list to CURSOR_REVIEW_CALLERS variable + mask repo names #37

New issue

Closed

opened 2026-10-26 22:35:37 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:35:37 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/27
Author: @mattmillerai
Created: 7/14/2026
Status: ✅ Merged
Merged: 7/18/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-3046-cursor-review-callers-var

📝 Commits (1)

a422331 ci(bump-callers): move caller list to CURSOR_REVIEW_CALLERS variable + mask repo names (BE-3046)

📊 Changes

1 file changed (+45 additions, -14 deletions)

View changed files

📝 .github/workflows/bump-cursor-review-callers.yml (+45 -14)

📄 Description

ELI-5

This workflow used to keep a list of every repo that runs cursor-review written right inside the file — and this repo is public, so anyone could read the list of our private repos, both in the file and in the (also public) run logs. This PR moves that list out of the file into a private Actions variable the workflow reads at run time, and blacks out (***) every repo name in the logs. Same behavior, no more leaks. Adding a repo later is now a variable edit, not a public commit.

What changed

Removed the hardcoded CALLERS=( … ) array from the workflow.
The list now comes from the repo-level Actions variable CURSOR_REVIEW_CALLERS — a JSON array of {"repo","file","label"} objects — parsed with jq into the exact same repo|file|label tuples the existing bump_one() loop already consumes (no downstream changes). A variable, not a secret: this is config, and secrets are write-only via the API.
Every repo name is ::add-mask::ed immediately after parsing, before any loop that echoes it, so all per-repo output ("<repo>: PR opened", per-repo warnings, the final failure summary) shows *** in the public logs instead of the repo name.
Hard-fails loudly (::error:: + exit 1) if the variable is missing, empty/whitespace, not a JSON array, empty array, or has any entry lacking a non-empty repo/file — a silent no-op dispatcher is not possible.
Documented the update flow (gh variable set CURSOR_REVIEW_CALLERS …) in the header, and noted the canonical callers.json should live in a private infra/ops repo as a reviewed source of truth.

Verification

python3 yaml.safe_load — YAML valid.
shellcheck -S warning on the extracted run script — clean (exit 0).
Unit-tested the validation/parse/mask block locally across every branch: empty, whitespace-only, invalid JSON, object-not-array, empty array, entry missing repo, entry with empty repo → all hard-fail with exit 1; valid input → parses, emits an ::add-mask:: per repo, and produces correct tuples (incl. missing-label → empty label).
Ran the actual seeded CURSOR_REVIEW_CALLERS variable through the parser: it yields exactly the 12 tuples, byte-for-byte identical to the previous hardcoded list plus Comfy-Org/evals (the addition from #25). No behavior change for bump_one().
Confirmed only Comfy-Org/github-workflows (this repo) appears anywhere in the file.

Judgment calls / notes for the reviewer

The variable is already seeded with the full 12-entry list (including evals), so acceptance-criterion 4's seed step is done. No further seeding needed.
AC #1 vs. the ticket's item 5 wording: the ticket suggested naming comfy-infra in the header as the canonical-JSON home, but comfy-infra is one of the private repos this ticket protects, and AC #1 says no repo name beyond github-workflows may appear in the file. AC #1 wins (it's the whole point), so the header says "a private infra/ops repo" generically rather than naming it.
AC #2 (a live workflow_dispatch run with clean logs) is a post-merge human step, not verified here — deliberately. workflow_dispatch runs the workflow version from the default branch, so this logic isn't dispatchable until it's merged; and dispatching the real workflow would open live SHA-bump PRs across all 12 caller repos (a side effect I won't trigger from a test). The masking is proven by inspection: every log statement echoes ${REPO} / ${FAILED[*]} (the full Comfy-Org/<repo> string), and each such string is ::add-mask::ed before the loop. Recommend one workflow_dispatch after merge to confirm the logs render ***.
#25 adds evals to the old hardcoded array, which this PR deletes. Since evals is already in the seeded variable, #25 is effectively superseded — recommend closing it (or merge it first and rebase this, either works; the variable already reflects the intended final state).
Masking nuance (benign): Comfy-Org/ComfyUI is a substring of Comfy-Org/ComfyUI_frontend, so masking can render the latter as ***/***_frontend — both are public repos, and none of the 8 private repos are substrings of one another, so no private name leaks.

_{🔄 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/27 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/14/2026 **Status:** ✅ Merged **Merged:** 7/18/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-3046-cursor-review-callers-var` --- ### 📝 Commits (1) - [`a422331`](https://github.com/Comfy-Org/github-workflows/commit/a4223314219a23a66b9d26749cc653ec0ff5c427) ci(bump-callers): move caller list to CURSOR_REVIEW_CALLERS variable + mask repo names (BE-3046) ### 📊 Changes **1 file changed** (+45 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `.github/workflows/bump-cursor-review-callers.yml` (+45 -14) </details> ### 📄 Description ## ELI-5 This workflow used to keep a list of every repo that runs cursor-review written **right inside the file** — and this repo is public, so anyone could read the list of our private repos, both in the file and in the (also public) run logs. This PR moves that list out of the file into a private Actions **variable** the workflow reads at run time, and blacks out (`***`) every repo name in the logs. Same behavior, no more leaks. Adding a repo later is now a variable edit, not a public commit. ## What changed - Removed the hardcoded `CALLERS=( … )` array from the workflow. - The list now comes from the repo-level Actions variable `CURSOR_REVIEW_CALLERS` — a JSON array of `{"repo","file","label"}` objects — parsed with `jq` into the exact same `repo|file|label` tuples the existing `bump_one()` loop already consumes (no downstream changes). A variable, not a secret: this is config, and secrets are write-only via the API. - Every repo name is `::add-mask::`ed immediately after parsing, **before** any loop that echoes it, so all per-repo output (`"<repo>: PR opened"`, per-repo warnings, the final failure summary) shows `***` in the public logs instead of the repo name. - **Hard-fails loudly** (`::error::` + `exit 1`) if the variable is missing, empty/whitespace, not a JSON array, empty array, or has any entry lacking a non-empty `repo`/`file` — a silent no-op dispatcher is not possible. - Documented the update flow (`gh variable set CURSOR_REVIEW_CALLERS …`) in the header, and noted the canonical `callers.json` should live in a private infra/ops repo as a reviewed source of truth. ## Verification - `python3 yaml.safe_load` — YAML valid. - `shellcheck -S warning` on the extracted run script — clean (exit 0). - Unit-tested the validation/parse/mask block locally across every branch: empty, whitespace-only, invalid JSON, object-not-array, empty array, entry missing `repo`, entry with empty `repo` → all hard-fail with `exit 1`; valid input → parses, emits an `::add-mask::` per repo, and produces correct tuples (incl. missing-`label` → empty label). - Ran the **actual seeded `CURSOR_REVIEW_CALLERS` variable** through the parser: it yields exactly the 12 tuples, **byte-for-byte identical** to the previous hardcoded list plus `Comfy-Org/evals` (the addition from #25). No behavior change for `bump_one()`. - Confirmed only `Comfy-Org/github-workflows` (this repo) appears anywhere in the file. ## Judgment calls / notes for the reviewer - **The variable is already seeded** with the full 12-entry list (including `evals`), so acceptance-criterion 4's seed step is done. No further seeding needed. - **AC #1 vs. the ticket's item 5 wording:** the ticket suggested naming `comfy-infra` in the header as the canonical-JSON home, but `comfy-infra` is one of the private repos this ticket protects, and AC #1 says *no* repo name beyond `github-workflows` may appear in the file. AC #1 wins (it's the whole point), so the header says "a private infra/ops repo" generically rather than naming it. - **AC #2 (a live `workflow_dispatch` run with clean logs) is a post-merge human step, not verified here — deliberately.** `workflow_dispatch` runs the workflow version from the default branch, so this logic isn't dispatchable until it's merged; and dispatching the *real* workflow would open live SHA-bump PRs across all 12 caller repos (a side effect I won't trigger from a test). The masking is proven by inspection: every log statement echoes `${REPO}` / `${FAILED[*]}` (the full `Comfy-Org/<repo>` string), and each such string is `::add-mask::`ed before the loop. Recommend one `workflow_dispatch` after merge to confirm the logs render `***`. - **#25** adds `evals` to the *old* hardcoded array, which this PR deletes. Since `evals` is already in the seeded variable, #25 is effectively superseded — recommend closing it (or merge it first and rebase this, either works; the variable already reflects the intended final state). - Masking nuance (benign): `Comfy-Org/ComfyUI` is a substring of `Comfy-Org/ComfyUI_frontend`, so masking can render the latter as `***`/`***_frontend` — both are *public* repos, and none of the 8 private repos are substrings of one another, so no private name leaks. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:35:37 +00:00