Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 56 additions & 15 deletions .github/bump-callers/bump-callers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,9 @@
# publicly viewable) and most callers are private, so caller names must never
# appear in a committed file or in the logs. Each caller list lives in a
# repo-level Actions variable (config, not a credential — a variable, not a
# secret) as a JSON array of {"repo","file","label"} objects, and every repo
# name is `::add-mask::`ed out of the run logs before it is ever echoed.
# secret) as a JSON array of {"repo","file","label","wire_bot"} objects, and
# every repo name is `::add-mask::`ed out of the run logs before it is ever
# echoed.
#
# Required environment:
# GH_TOKEN Token with contents:write + pull-requests:write on the callers
Expand All @@ -27,11 +28,19 @@
# TAG Human tag for branch/commit/PR text (e.g. "cursor-review").
# WORKFLOW_FILE Reusable workflow filename referenced in the PR body.
# Optional:
# ALLOW_EMPTY "true" → an empty caller list is a clean no-op (a fleet that
# is seeded empty and grows). Default "false" → an empty/missing
# list is a hard error (a fleet that always has callers must
# never silently no-op — that would leave every caller un-bumped
# without anyone noticing).
# ALLOW_EMPTY "true" → an empty caller list is a clean no-op (a fleet that
# is seeded empty and grows). Default "false" → an empty/missing
# list is a hard error (a fleet that always has callers must
# never silently no-op — that would leave every caller un-bumped
# without anyone noticing).
# WIRE_BOT_SCRIPT Path to a helper that reads a caller's YAML on stdin and
# writes it back wired for some extra identity/config, idempotently
# (BE-1814's .github/cursor-review/wire-bot-identity.py). Only
# consulted for an entry whose `wire_bot` field is true; unset
# (the agents-md-integrity entrypoint never sets it) means no
# fleet here uses wiring, so a stray `wire_bot: true` in that
# fleet's variable is a harmless no-op with a warning, not a
# failure — this keeps the script itself cursor-review-agnostic.

# NOTE: deliberately no `set -e`. Each caller is bumped independently in
# bump_one(); a failure there returns non-zero and is downgraded to a per-repo
Expand All @@ -45,6 +54,7 @@ set -uo pipefail
: "${WORKFLOW_FILE:?WORKFLOW_FILE is required}"
CALLERS_JSON="${CALLERS_JSON-}"
ALLOW_EMPTY="${ALLOW_EMPTY:-false}"
WIRE_BOT_SCRIPT="${WIRE_BOT_SCRIPT-}"

SHORT="${NEW_SHA:0:7}"
# Stable branch per (repo, TAG) — deliberately NOT SHA-stamped. A fixed head
Expand All @@ -70,19 +80,21 @@ if [[ -z "$STRIPPED" ]]; then
echo "::error::${VAR_NAME} variable is missing or empty. Seed it with the caller list — see this workflow's header comment for the update flow."
exit 1
fi
if ! jq -e 'type == "array" and length > 0 and all(.[]; (.repo | type == "string" and . != "") and (.file | type == "string" and . != ""))' <<<"$CALLERS_JSON" >/dev/null 2>&1; then
echo "::error::${VAR_NAME} is not a non-empty JSON array of {repo,file,label} objects (each needing a non-empty repo and file). Fix the variable — see the workflow header."
if ! jq -e 'type == "array" and length > 0 and all(.[]; (.repo | type == "string" and . != "") and (.file | type == "string" and . != "") and (.wire_bot == null or (.wire_bot | type == "boolean")))' <<<"$CALLERS_JSON" >/dev/null 2>&1; then
echo "::error::${VAR_NAME} is not a non-empty JSON array of {repo,file,label,wire_bot} objects (each needing a non-empty repo and file, and a boolean wire_bot if present). Fix the variable — see the workflow header."
exit 1
fi

# Parse the JSON into repo|file|label tuples, and mask every repo name in the
# (publicly viewable) run logs BEFORE the loop that echoes it, so all per-repo
# output shows *** instead of a private repo name.
# Parse the JSON into repo|file|label|wire_bot tuples, and mask every repo name
# in the (publicly viewable) run logs BEFORE the loop that echoes it, so all
# per-repo output shows *** instead of a private repo name. jq's `//` treats
# both `false` and `null` as falsy, so an absent/false/null wire_bot all print
# the empty string here — exactly the "not flagged" case.
CALLERS=()
while IFS= read -r ENTRY; do
echo "::add-mask::${ENTRY%%|*}"
CALLERS+=("$ENTRY")
done < <(jq -r '.[] | "\(.repo)|\(.file)|\(.label // "")"' <<<"$CALLERS_JSON")
done < <(jq -r '.[] | "\(.repo)|\(.file)|\(.label // "")|\(.wire_bot // "")"' <<<"$CALLERS_JSON")

if (( ${#CALLERS[@]} == 0 )); then
echo "::error::${VAR_NAME} parsed to zero callers — refusing to run a no-op dispatcher."
Expand Down Expand Up @@ -128,10 +140,12 @@ bump_repo() {
# files are all missing or already pinned (the old per-entry skips), so it
# never resets a branch or opens a PR it doesn't need.
local -a PEND_FILE_ENC=() PEND_CONTENT=() PEND_BLOB=() LABELS=()
local ENTRY FILE LABEL FILE_ENC CURRENT BLOB_SHA OLD_CONTENT NEW_CONTENT
local WIRING_ADDED_ANY=""
local ENTRY FILE LABEL WIRE_BOT FILE_ENC CURRENT BLOB_SHA OLD_CONTENT NEW_CONTENT
for ENTRY in "$@"; do
FILE=$(cut -d'|' -f2 <<<"$ENTRY")
LABEL=$(cut -d'|' -f3 <<<"$ENTRY")
WIRE_BOT=$(cut -d'|' -f4 <<<"$ENTRY")
FILE_ENC="${FILE//\//%2F}"

# De-duplicate by file: a repo listed twice for the SAME path (a structurally
Expand Down Expand Up @@ -183,6 +197,27 @@ bump_repo() {
# form (e.g. agents-md-integrity's `# v1`), so it is safe to share.
NEW_CONTENT=$(sed -E "/github-workflows|workflows_ref/ s/[0-9a-f]{40}/${NEW_SHA}/g; s|# github-workflows#[0-9]+|# github-workflows main (${SHORT})|g" <<<"$OLD_CONTENT")

# Wire an extra identity/config into this file when its entry is flagged
# (BE-1814's cloud-code-bot review identity is the first user). Idempotent —
# an already-wired caller comes back unchanged — so this only adds the
# wiring to callers that don't have it yet, alongside the SHA bump. Folded
# into the SAME content-equality check below (not a separate "grep for the
# wired marker" skip) so a wiring-only change on an already-SHA-current
# caller still stages the file instead of being short-circuited.
if [[ -n "$WIRE_BOT" ]]; then
if [[ -z "$WIRE_BOT_SCRIPT" ]]; then
echo "::warning::${REPO}: ${FILE} flagged wire_bot but WIRE_BOT_SCRIPT is unset — bumping SHA only"
else
local WIRED
if WIRED=$(python3 "$WIRE_BOT_SCRIPT" <<<"$NEW_CONTENT") && [[ -n "$WIRED" ]]; then
[[ "$WIRED" != "$NEW_CONTENT" ]] && WIRING_ADDED_ANY=1
NEW_CONTENT="$WIRED"
else
echo "::warning::${REPO}: ${FILE} bot-identity wiring failed — bumping SHA only"
fi
fi
fi

# Already fully pinned → the rewrite is a no-op → nothing to do for this file.
# Comparing the rewritten content to the original (rather than grepping for
# NEW_SHA appearing *anywhere*) also repairs a half-bumped file: if a prior
Expand Down Expand Up @@ -253,7 +288,13 @@ bump_repo() {
# YAML in an earlier revision.
local PR_TITLE PR_BODY
PR_TITLE="ci: bump ${TAG} to github-workflows@${SHORT}"
PR_BODY="Automatic SHA bump — \`${WORKFLOW_FILE}\` was updated in \`Comfy-Org/github-workflows\` at [\`${SHORT}\`](https://github.com/Comfy-Org/github-workflows/commit/${NEW_SHA}). _Opened by the \`bump-${TAG}-callers\` workflow._"
PR_BODY="Automatic SHA bump — \`${WORKFLOW_FILE}\` was updated in \`Comfy-Org/github-workflows\` at [\`${SHORT}\`](https://github.com/Comfy-Org/github-workflows/commit/${NEW_SHA})."
# Note the identity wiring in the PR body when it was actually added this run
# (skipped for a caller already wired — that gets a pure SHA bump). NB: keep
# GitHub-Actions expression syntax out of this script — the caller entrypoint
# interpolates those at parse time, not here.
[[ -n "$WIRING_ADDED_ANY" ]] && PR_BODY="${PR_BODY} Also wires the cloud-code-bot review identity (\`bot_app_id\` → \`vars.APP_ID\`, plus the \`BOT_APP_PRIVATE_KEY\` secret) so the review posts under \`cloud-code-bot[bot]\` instead of \`github-actions[bot]\` — BE-1814."
PR_BODY="${PR_BODY} _Opened by the \`bump-${TAG}-callers\` workflow._"

# Reuse the one open bump PR for this branch if there is one: the branch push
# above already refreshed its diff to the new SHA, so just refresh its
Expand Down
74 changes: 74 additions & 0 deletions .github/bump-callers/tests/test_bump_callers.sh
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,80 @@ check "ignored the fork PR, opened the real one" "grep -q 'PR opened' <<<\"\$OU
check "did NOT edit the attacker's fork PR" "! grep -q '^pr-edit 1337' \"\$STUB_PUT_DIR/pr.log\""
check "opened a fresh PR via create" "grep -q '^pr-create' \"\$STUB_PUT_DIR/pr.log\""

echo "== cursor-review fleet: wire_bot=true also wires the cloud-code-bot identity (BE-1814) =="
# The real wire-bot-identity.py helper, driven end to end (no stub) — a caller
# flagged wire_bot must get BOTH the SHA bump AND the identity wired in one PR.
WIRE_SCRIPT="${SCRIPT_DIR}/../../cursor-review/wire-bot-identity.py"
WIRE_FIXTURE="${WORK}/wire_caller.yml"
printf '%s\n' \
'name: CI cursor-review' \
'jobs:' \
' review:' \
' uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@1111111111111111111111111111111111111111 # github-workflows#27' \
' with:' \
' pr_number: 42' \
' secrets:' \
' CURSOR_API_KEY: dummy' \
> "$WIRE_FIXTURE"
new_case wire
STUB_CONTENT_FILE="$WIRE_FIXTURE" WIRE_BOT_SCRIPT="$WIRE_SCRIPT" run_bump \
VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \
CALLERS_JSON='[{"repo":"Comfy-Org/secret-wired","file":".github/workflows/ci-cursor-review.yml","label":"","wire_bot":true}]'
check "exit 0" "[[ $RC -eq 0 ]]"
PUT="${STUB_PUT_DIR}/put.last.txt"
check "SHA bumped" "grep -qF '$NEW_SHA' \"$PUT\""
check "bot_app_id wired in" "grep -q 'bot_app_id: \${{ vars.APP_ID }}' \"$PUT\""
check "BOT_APP_PRIVATE_KEY wired in" "grep -q 'BOT_APP_PRIVATE_KEY: \${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }}' \"$PUT\""
check "PR body notes the wiring" "grep -q 'BE-1814' \"\$STUB_PUT_DIR/pr.log\""
check "reported fleet complete" "grep -q 'cursor-review bump complete' <<<\"\$OUT\""

echo "== cursor-review fleet: wire_bot=false (default) never wires, even with WIRE_BOT_SCRIPT set =="
new_case nowire
STUB_CONTENT_FILE="$WIRE_FIXTURE" WIRE_BOT_SCRIPT="$WIRE_SCRIPT" run_bump \
VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \
CALLERS_JSON='[{"repo":"Comfy-Org/secret-unwired","file":".github/workflows/ci-cursor-review.yml","label":""}]'
check "exit 0" "[[ $RC -eq 0 ]]"
PUT="${STUB_PUT_DIR}/put.last.txt"
check "SHA still bumped" "grep -qF '$NEW_SHA' \"$PUT\""
check "bot_app_id NOT wired in" "! grep -q 'bot_app_id:' \"$PUT\""
check "BOT_APP_PRIVATE_KEY NOT wired in" "! grep -q 'BOT_APP_PRIVATE_KEY:' \"$PUT\""

echo "== cursor-review fleet: wire_bot=true but WIRE_BOT_SCRIPT unset degrades to SHA-bump-only =="
new_case wirenoscript
STUB_CONTENT_FILE="$WIRE_FIXTURE" run_bump \
VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \
CALLERS_JSON='[{"repo":"Comfy-Org/secret-noscript","file":".github/workflows/ci-cursor-review.yml","label":"","wire_bot":true}]'
check "exit 0 (degrades, does not fail the repo)" "[[ $RC -eq 0 ]]"
check "warned WIRE_BOT_SCRIPT is unset" "grep -q 'WIRE_BOT_SCRIPT is unset' <<<\"\$OUT\""
PUT="${STUB_PUT_DIR}/put.last.txt"
check "SHA still bumped" "grep -qF '$NEW_SHA' \"$PUT\""
check "bot_app_id NOT wired in" "! grep -q 'bot_app_id:' \"$PUT\""

echo "== cursor-review fleet: already-wired + already-current caller is a clean skip (Chesterton's Fence) =="
# A caller that already has the wiring AND is already at the target SHA must be
# a true no-op — the content-equality check (not a bare SHA grep) is what makes
# a wiring-only change on an already-current caller still stage, while a
# fully-converged caller (this case) stays a clean skip.
ALREADY_WIRED_FIXTURE="${WORK}/already_wired_caller.yml"
printf '%s\n' \
'name: CI cursor-review' \
'jobs:' \
' review:' \
" uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@${NEW_SHA} # github-workflows main (${SHORT})" \
' with:' \
' bot_app_id: dummy' \
' secrets:' \
' CURSOR_API_KEY: dummy' \
' BOT_APP_PRIVATE_KEY: dummy' \
> "$ALREADY_WIRED_FIXTURE"
new_case alreadywired
STUB_CONTENT_FILE="$ALREADY_WIRED_FIXTURE" WIRE_BOT_SCRIPT="$WIRE_SCRIPT" run_bump \
VAR_NAME=CURSOR_REVIEW_CALLERS TAG=cursor-review WORKFLOW_FILE=cursor-review.yml \
CALLERS_JSON='[{"repo":"Comfy-Org/secret-converged","file":".github/workflows/ci-cursor-review.yml","label":"","wire_bot":true}]'
check "exit 0" "[[ $RC -eq 0 ]]"
check "reported already at SHORT (+ wired)" "grep -q 'already at $SHORT' <<<\"\$OUT\""
check "committed nothing" "[[ ! -f \"\$STUB_PUT_DIR/count\" ]]"

echo "== agents-md fleet: two callers, two SHA refs, '# v1' preserved =="
new_case amd
AMD_FIXTURE="${WORK}/amd_caller.yml"
Expand Down
131 changes: 131 additions & 0 deletions .github/cursor-review/tests/test_wire_bot_identity.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
#!/usr/bin/env python3
"""Regression tests for wire-bot-identity.py.

The helper injects the cloud-code-bot identity (bot_app_id input +
BOT_APP_PRIVATE_KEY secret) into a cursor-review caller as the fan-out step of
BE-1814. The properties that matter — and that these tests pin — are:

* the two anchors get exactly the ticket's mapping (vars.APP_ID /
secrets.CLOUD_CODE_BOT_PRIVATE_KEY),
* injection is idempotent (an already-wired caller is a byte-for-byte no-op),
* only the wiring changes — comments, folded diff_excludes, and the SHA-pin
line are preserved (a PyYAML round-trip would destroy them), and
* indentation is inherited from the caller, not hard-coded.

Run: python3 .github/cursor-review/tests/test_wire_bot_identity.py
"""

import importlib.util
import os
import unittest

# wire-bot-identity.py has a hyphen, so import it by path rather than `import`.
_MODULE_PATH = os.path.join(os.path.dirname(__file__), "..", "wire-bot-identity.py")
_spec = importlib.util.spec_from_file_location("wire_bot_identity", _MODULE_PATH)
wbi = importlib.util.module_from_spec(_spec)
_spec.loader.exec_module(wbi)


# A representative unwired caller (comfy-inapp-agent's shape: `with:` + `secrets:`
# both present, no bot identity yet).
UNWIRED = """\
jobs:
cursor-review:
permissions:
contents: read
pull-requests: write
# SHA-pinned per zizmor `unpinned-uses: hash-pin`.
uses: Comfy-Org/github-workflows/.github/workflows/cursor-review.yml@df507e6bae179c567ad3849370f99dae588985dc # github-workflows main (df507e6)
with:
workflows_ref: df507e6bae179c567ad3849370f99dae588985dc
# Minimal excludes for a small Node + TS extension.
diff_excludes: >-
:!**/package-lock.json
:!**/node_modules/**
secrets:
CURSOR_API_KEY: ${{ secrets.CURSOR_API_KEY }}
SLACK_BOT_TOKEN: ${{ secrets.SLACK_BOT_TOKEN }}
"""


class WireBotIdentityTest(unittest.TestCase):
def test_injects_both_anchors_with_exact_mapping(self):
out = wbi.wire(UNWIRED)
self.assertIn("bot_app_id: ${{ vars.APP_ID }}", out)
self.assertIn(
"BOT_APP_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }}", out
)

def test_bot_app_id_nested_under_with_not_secrets(self):
out = wbi.wire(UNWIRED).split("\n")
with_idx = next(i for i, l in enumerate(out) if l.strip() == "with:")
secrets_idx = next(i for i, l in enumerate(out) if l.strip() == "secrets:")
app_idx = next(i for i, l in enumerate(out) if "bot_app_id:" in l)
key_idx = next(i for i, l in enumerate(out) if "BOT_APP_PRIVATE_KEY:" in l)
# bot_app_id sits inside the with: block; the private key inside secrets:.
self.assertTrue(with_idx < app_idx < secrets_idx)
self.assertTrue(secrets_idx < key_idx)

def test_inherits_child_indentation(self):
out = wbi.wire(UNWIRED).split("\n")
app_line = next(l for l in out if "bot_app_id:" in l)
key_line = next(l for l in out if "BOT_APP_PRIVATE_KEY:" in l)
# `with:`/`secrets:` are at 4 spaces, so children land at 6.
self.assertTrue(app_line.startswith(" bot_app_id:"))
self.assertTrue(key_line.startswith(" BOT_APP_PRIVATE_KEY:"))

def test_idempotent_on_already_wired(self):
once = wbi.wire(UNWIRED)
twice = wbi.wire(once)
self.assertEqual(once, twice)

def test_already_wired_is_exact_no_op(self):
# An already-wired caller must be returned byte-for-byte unchanged.
already = wbi.wire(UNWIRED)
self.assertEqual(wbi.wire(already), already)

def test_preserves_comments_and_diff_excludes(self):
out = wbi.wire(UNWIRED)
self.assertIn("# SHA-pinned per zizmor", out)
self.assertIn("diff_excludes: >-", out)
self.assertIn(":!**/node_modules/**", out)
self.assertIn("# github-workflows main (df507e6)", out)
# Every original line survives (only additions, no deletions/edits).
for line in UNWIRED.split("\n"):
self.assertIn(line, out.split("\n"))

def test_partial_wire_completes_the_missing_half(self):
# Caller that already has bot_app_id but is missing the secret.
half = UNWIRED.replace(
" workflows_ref:",
" bot_app_id: ${{ vars.APP_ID }}\n workflows_ref:",
1,
)
out = wbi.wire(half)
# The secret is added...
self.assertIn(
"BOT_APP_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }}", out
)
# ...and bot_app_id is not duplicated.
self.assertEqual(out.count("bot_app_id:"), 1)

def test_only_wiring_lines_are_added(self):
before = UNWIRED.split("\n")
after = wbi.wire(UNWIRED).split("\n")
added = [l for l in after if l not in before]
# Only the two key lines + their explanatory comments are new; every
# added line is a comment or one of the two wiring keys.
for line in added:
stripped = line.strip()
self.assertTrue(
stripped.startswith("#")
or stripped.startswith("bot_app_id:")
or stripped.startswith("BOT_APP_PRIVATE_KEY:"),
f"unexpected added line: {line!r}",
)
self.assertTrue(any("bot_app_id:" in l for l in added))
self.assertTrue(any("BOT_APP_PRIVATE_KEY:" in l for l in added))


if __name__ == "__main__":
unittest.main(verbosity=2)
Loading