From 4758456b0662ae69eaf1340acb25476bb8063928 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Thu, 16 Jul 2026 13:13:33 -0700 Subject: [PATCH 1/2] ci(bump-callers): wire cloud-code-bot review identity into flagged callers (BE-1814) Story 1.3 of BE-1800: fan out the optional bot-identity wiring the reusable cursor-review.yml gained in Story 1.1 (PR #13) through the caller-bump workflow instead of hand-editing each consumer. - New idempotent helper .github/cursor-review/wire-bot-identity.py injects bot_app_id: ${{ vars.APP_ID }} under with: and BOT_APP_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} under secrets:, preserving comments/formatting (line-based, not a YAML round-trip). - bump-cursor-review-callers.yml gains a per-caller wire_bot flag (4th field); flagged only for repos that already hold the creds (cloud, mcp-server, comfy-inapp-agent, Comfy-iOS). Others stay github-actions[bot] (non-breaking). - Skip check is now content-based so a wiring-only change still opens a PR. - Unit tests for the helper run under the existing test-cursor-review-scripts CI. --- .../tests/test_wire_bot_identity.py | 131 +++++++++++++++++ .github/cursor-review/wire-bot-identity.py | 133 ++++++++++++++++++ .../workflows/bump-cursor-review-callers.yml | 99 ++++++++++--- 3 files changed, 343 insertions(+), 20 deletions(-) create mode 100644 .github/cursor-review/tests/test_wire_bot_identity.py create mode 100644 .github/cursor-review/wire-bot-identity.py diff --git a/.github/cursor-review/tests/test_wire_bot_identity.py b/.github/cursor-review/tests/test_wire_bot_identity.py new file mode 100644 index 0000000..5d31def --- /dev/null +++ b/.github/cursor-review/tests/test_wire_bot_identity.py @@ -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) diff --git a/.github/cursor-review/wire-bot-identity.py b/.github/cursor-review/wire-bot-identity.py new file mode 100644 index 0000000..8e5d430 --- /dev/null +++ b/.github/cursor-review/wire-bot-identity.py @@ -0,0 +1,133 @@ +#!/usr/bin/env python3 +"""Idempotently wire the cloud-code-bot identity into a cursor-review caller. + +Story 1.3 (BE-1814): the reusable cursor-review.yml (Story 1.1, PR #13) takes an +optional GitHub App identity so its consolidated review + line comments post +under a dedicated bot login instead of github-actions[bot]. Each consumer that +already holds the cloud-code-bot creds maps them through its thin caller: + + with: + bot_app_id: ${{ vars.APP_ID }} + secrets: + BOT_APP_PRIVATE_KEY: ${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }} + +Rather than hand-edit each caller, bump-cursor-review-callers.yml pipes the file +through this helper (gated per-caller — only repos with the creds provisioned). + +Why line-based and not a PyYAML round-trip: the callers are heavily commented and +carry a specific hand-tuned layout (folded `diff_excludes`, inline SHA-pin +comments). A yaml.load/dump would strip every comment and reflow the file, so the +fan-out PR would be an unreviewable rewrite. This edits only the two anchor points +and leaves the rest byte-for-byte, so the PR diff is exactly the wiring. + +Idempotent: if `bot_app_id:` / `BOT_APP_PRIVATE_KEY:` are already present, that +half is left untouched — a repo already wired (or re-run) converges to a no-op. + +Usage: reads the caller YAML on stdin, writes the wired YAML to stdout. + python3 wire-bot-identity.py < caller.yml > wired.yml +""" + +import re +import sys + +APP_ID_VALUE = "${{ vars.APP_ID }}" +PRIVATE_KEY_VALUE = "${{ secrets.CLOUD_CODE_BOT_PRIVATE_KEY }}" + +_WITH_RE = re.compile(r"^(\s*)with:\s*$") +_SECRETS_RE = re.compile(r"^(\s*)secrets:\s*$") +_BOT_APP_ID_RE = re.compile(r"^\s*bot_app_id\s*:") +_PRIVATE_KEY_RE = re.compile(r"^\s*BOT_APP_PRIVATE_KEY\s*:") +_CURSOR_API_KEY_RE = re.compile(r"^(\s*)CURSOR_API_KEY\s*:") + + +def _leading_ws(line: str) -> str: + return line[: len(line) - len(line.lstrip(" "))] + + +def _child_indent(lines, block_idx: int, block_indent: str) -> str: + """Indent of the first child of the block header at ``block_idx``. + + Falls back to the header's indent + 2 spaces when the block has no existing + child to copy the indentation from. + """ + for line in lines[block_idx + 1:]: + if not line.strip(): + continue + indent = _leading_ws(line) + if len(indent) > len(block_indent): + return indent + # Dedented to the block's level or shallower — block has no children. + break + return block_indent + " " + + +def wire(text: str) -> str: + """Return ``text`` with the cloud-code-bot identity wired in (idempotent).""" + lines = text.split("\n") + + already_app_id = any(_BOT_APP_ID_RE.match(ln) for ln in lines) + already_key = any(_PRIVATE_KEY_RE.match(ln) for ln in lines) + if already_app_id and already_key: + return text + + # --- inject bot_app_id as the first child of the job's `with:` block --- + if not already_app_id: + for i, line in enumerate(lines): + m = _WITH_RE.match(line) + if not m: + continue + indent = _child_indent(lines, i, m.group(1)) + block = [ + f"{indent}# Post the consolidated review + line comments under the cloud-code-bot", + f"{indent}# app identity (vars.APP_ID) instead of github-actions[bot]; the paired", + f"{indent}# key rides the BOT_APP_PRIVATE_KEY secret below. Both optional — absent", + f"{indent}# creds fall back to github-actions[bot] (non-breaking).", + f"{indent}bot_app_id: {APP_ID_VALUE}", + ] + lines[i + 1:i + 1] = block + break + else: + sys.stderr.write( + "wire-bot-identity: no `with:` block found — bot_app_id not wired\n" + ) + + # --- inject BOT_APP_PRIVATE_KEY under the job's `secrets:` block --- + if not already_key: + insert_at = None + secret_indent = None + # Prefer to anchor right after CURSOR_API_KEY so it sits beside its siblings. + for i, line in enumerate(lines): + m = _CURSOR_API_KEY_RE.match(line) + if m: + insert_at = i + 1 + secret_indent = m.group(1) + break + if insert_at is None: + for i, line in enumerate(lines): + m = _SECRETS_RE.match(line) + if m: + secret_indent = _child_indent(lines, i, m.group(1)) + insert_at = i + 1 + break + if insert_at is not None: + block = [ + f"{secret_indent}# PEM key paired with bot_app_id (vars.APP_ID). Optional — see above.", + f"{secret_indent}BOT_APP_PRIVATE_KEY: {PRIVATE_KEY_VALUE}", + ] + lines[insert_at:insert_at] = block + else: + sys.stderr.write( + "wire-bot-identity: no `secrets:` block found — " + "BOT_APP_PRIVATE_KEY not wired\n" + ) + + return "\n".join(lines) + + +def main() -> int: + sys.stdout.write(wire(sys.stdin.read())) + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/.github/workflows/bump-cursor-review-callers.yml b/.github/workflows/bump-cursor-review-callers.yml index 4f9d730..4637be5 100644 --- a/.github/workflows/bump-cursor-review-callers.yml +++ b/.github/workflows/bump-cursor-review-callers.yml @@ -3,6 +3,15 @@ name: Bump cursor-review callers # When cursor-review.yml is updated on main, open a SHA-bump PR in every repo # that pins a caller against it. PRs are opened by Cloud Code Bot so they are # easy to filter and merge. +# +# Callers flagged `wire_bot` (4th field) also get the cloud-code-bot review +# identity wired in the same PR (BE-1814): the optional bot_app_id input + +# BOT_APP_PRIVATE_KEY secret the reusable workflow gained in Story 1.1 (PR #13), +# mapped to the creds those repos already hold (vars.APP_ID / +# secrets.CLOUD_CODE_BOT_PRIVATE_KEY). Injection is idempotent — see +# .github/cursor-review/wire-bot-identity.py — so an already-wired caller only +# gets the SHA bump. Flag ONLY repos that have the creds provisioned; a repo +# without them stays github-actions[bot] (non-breaking, incremental rollout). on: workflow_dispatch: {} # allow on-demand runs (e.g. to re-bump callers) @@ -15,6 +24,14 @@ jobs: bump: runs-on: ubuntu-latest steps: + - name: Checkout + # Needed for the wire-bot-identity.py helper the bump step pipes callers + # through. persist-credentials:false — the bump uses the App token via + # GH_TOKEN, not the checkout's git credentials. + uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + - name: Generate Cloud Code Bot token uses: actions/create-github-app-token@d72941d797fd3113feb6b93fd0dec494b13a2547 # v1.12.0 id: token @@ -36,19 +53,34 @@ jobs: SHORT="${NEW_SHA:0:7}" BRANCH="ci/bump-cursor-review-${SHORT}" - # repo|file|label — label is optional, leave empty if the repo has none + # repo|file|label|wire_bot + # label — optional PR label; leave empty if the repo has none. + # wire_bot — "1" to also wire the cloud-code-bot review identity in + # the same PR (BE-1814): the optional bot_app_id input + + # BOT_APP_PRIVATE_KEY secret from Story 1.1 (PR #13), + # mapped to vars.APP_ID / secrets.CLOUD_CODE_BOT_PRIVATE_KEY. + # Empty = SHA-bump only; the caller stays github-actions[bot]. + # ⚠ Flag ONLY repos that already hold the creds — a caller that + # references a missing secret still degrades to github-actions[bot] + # (non-breaking), but flagging a repo without the app is misleading. + # Wired: cloud + mcp-server (already wired — injection is a no-op) + # and comfy-inapp-agent + Comfy-iOS (their cursor-review-auto-label + # callers already use the same app, so the creds are provisioned). + # NOT: comfy-local-mcp / ComfySwiftSDK / comfy-infra / the public + # OSS repos (no cloud-code-bot creds), and NOT swarmhost (its own + # caller notes the app is not provisioned there yet). CALLERS=( - "Comfy-Org/cloud|.github/workflows/ci-cursor-review.yml|" - "Comfy-Org/comfy-cloud-mcp-server|.github/workflows/ci-cursor-review.yml|" - "Comfy-Org/comfy-local-mcp|.github/workflows/ci-cursor-review.yml|" - "Comfy-Org/Comfy-iOS|.github/workflows/ci-cursor-review.yml|" - "Comfy-Org/ComfySwiftSDK|.github/workflows/ci-cursor-review.yml|" - "Comfy-Org/comfy-inapp-agent|.github/workflows/ci-cursor-review.yml|" - "Comfy-Org/swarmhost|.github/workflows/cursor-review.yml|" - "Comfy-Org/comfy-infra|.github/workflows/ci-cursor-review.yml|" - "Comfy-Org/ComfyUI_frontend|.github/workflows/pr-cursor-review.yaml|" - "Comfy-Org/Comfy-Desktop|.github/workflows/ci-cursor-review.yml|ci" - "Comfy-Org/ComfyUI|.github/workflows/ci-cursor-review.yml|Core" + "Comfy-Org/cloud|.github/workflows/ci-cursor-review.yml||1" + "Comfy-Org/comfy-cloud-mcp-server|.github/workflows/ci-cursor-review.yml||1" + "Comfy-Org/comfy-local-mcp|.github/workflows/ci-cursor-review.yml||" + "Comfy-Org/Comfy-iOS|.github/workflows/ci-cursor-review.yml||1" + "Comfy-Org/ComfySwiftSDK|.github/workflows/ci-cursor-review.yml||" + "Comfy-Org/comfy-inapp-agent|.github/workflows/ci-cursor-review.yml||1" + "Comfy-Org/swarmhost|.github/workflows/cursor-review.yml||" + "Comfy-Org/comfy-infra|.github/workflows/ci-cursor-review.yml||" + "Comfy-Org/ComfyUI_frontend|.github/workflows/pr-cursor-review.yaml||" + "Comfy-Org/Comfy-Desktop|.github/workflows/ci-cursor-review.yml|ci|" + "Comfy-Org/ComfyUI|.github/workflows/ci-cursor-review.yml|Core|" ) # Bump a single caller repo. Every external call is guarded, so a bad @@ -56,10 +88,11 @@ jobs: # the run. An expected skip (file missing / already pinned) is a # successful `return 0`. bump_one() { - local ENTRY="$1" REPO FILE LABEL FILE_ENC + local ENTRY="$1" REPO FILE LABEL WIRE_BOT FILE_ENC REPO=$(cut -d'|' -f1 <<<"$ENTRY") FILE=$(cut -d'|' -f2 <<<"$ENTRY") LABEL=$(cut -d'|' -f3 <<<"$ENTRY") + WIRE_BOT=$(cut -d'|' -f4 <<<"$ENTRY") FILE_ENC="${FILE//\//%2F}" local DEFAULT_BRANCH @@ -79,16 +112,34 @@ jobs: BLOB_SHA=$(jq -r '.sha' <<<"$CURRENT") OLD_CONTENT=$(jq -r '.content' <<<"$CURRENT" | base64 -d) - # Already pinned to this SHA → nothing to do (success). - if grep -qF "$NEW_SHA" <<<"$OLD_CONTENT"; then - echo "${REPO}: already at ${SHORT} — skipping" - return 0 - fi - # Replace every 40-char hex SHA and update the inline comment. local NEW_CONTENT NEW_CONTENT=$(sed -E "s/[0-9a-f]{40}/${NEW_SHA}/g; s|# github-workflows#[0-9]+|# github-workflows main (${SHORT})|g" <<<"$OLD_CONTENT") + # Wire the cloud-code-bot review identity for flagged callers + # (BE-1814). Idempotent — an already-wired caller is unchanged — so + # this only adds the bot_app_id input + BOT_APP_PRIVATE_KEY secret to + # callers that don't have them yet, alongside the SHA bump. + local WIRING_ADDED="" + if [[ -n "$WIRE_BOT" ]]; then + local WIRED + if WIRED=$(python3 "${GITHUB_WORKSPACE}/.github/cursor-review/wire-bot-identity.py" <<<"$NEW_CONTENT") && [[ -n "$WIRED" ]]; then + [[ "$WIRED" != "$NEW_CONTENT" ]] && WIRING_ADDED=1 + NEW_CONTENT="$WIRED" + else + echo "::warning::${REPO}: bot-identity wiring failed — bumping SHA only" + fi + fi + + # Nothing changed (already at this SHA and, if flagged, already + # wired) → no-op, skip. This content check replaces the old + # "grep for NEW_SHA" skip so a wiring-only change (SHA already + # current) still opens a PR instead of being short-circuited. + if [[ "$NEW_CONTENT" == "$OLD_CONTENT" ]]; then + echo "${REPO}: already at ${SHORT}$([[ -n "$WIRE_BOT" ]] && echo ' + wired') — skipping" + return 0 + fi + # Create the branch from the default-branch tip (ignore 422 if it exists). local MAIN_SHA MAIN_SHA=$(gh api "repos/${REPO}/git/refs/heads/${DEFAULT_BRANCH}" --jq '.object.sha') || { @@ -119,13 +170,21 @@ jobs: local LABEL_ARGS=() [[ -n "$LABEL" ]] && LABEL_ARGS=(--label "$LABEL") + # Note the identity wiring in the PR body when it was actually added + # (skipped for callers already wired — those get a pure SHA bump). + local BODY="Automatic SHA bump — \`cursor-review.yml\` was updated in \`Comfy-Org/github-workflows\` at [\`${SHORT}\`](https://github.com/Comfy-Org/github-workflows/commit/${NEW_SHA})." + # NB: keep GitHub-Actions expression syntax out of this run block — + # Actions interpolates those at parse time. Describe in prose instead. + [[ -n "$WIRING_ADDED" ]] && BODY="${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." + BODY="${BODY} _Opened by the \`bump-cursor-review-callers\` workflow._" + # Open the PR (a pre-existing PR for this branch is not an error). if gh pr create \ --repo "${REPO}" \ --head "${BRANCH}" \ --base "${DEFAULT_BRANCH}" \ --title "ci: bump cursor-review to github-workflows@${SHORT}" \ - --body "Automatic SHA bump — \`cursor-review.yml\` was updated in \`Comfy-Org/github-workflows\` at [\`${SHORT}\`](https://github.com/Comfy-Org/github-workflows/commit/${NEW_SHA}). _Opened by the \`bump-cursor-review-callers\` workflow._" \ + --body "$BODY" \ "${LABEL_ARGS[@]}" 2>/dev/null; then echo "${REPO}: PR opened" else From cd1181d65ba64fb4e94d5f4894e5bbe97eb71871 Mon Sep 17 00:00:00 2001 From: Matt Miller Date: Tue, 21 Jul 2026 10:46:12 -0700 Subject: [PATCH 2/2] feat(bump-callers): teach the shared bumper the wire_bot field (BE-1814) main refactored bump-cursor-review-callers.yml to a JSON caller variable + shared bump-callers.sh while this branch was adding wire_bot support to the old hardcoded-CALLERS version. Re-implement the capability on the new architecture instead of reviving the old one: bump-callers.sh now accepts an optional wire_bot field per caller entry and an optional WIRE_BOT_SCRIPT env (set only by bump-cursor-review-callers.yml, pointing at wire-bot-identity.py), so the identity wiring folds into the same content-equality staging pass as the SHA rewrite. --- .github/bump-callers/bump-callers.sh | 71 ++++++++++++++---- .../bump-callers/tests/test_bump_callers.sh | 74 +++++++++++++++++++ 2 files changed, 130 insertions(+), 15 deletions(-) diff --git a/.github/bump-callers/bump-callers.sh b/.github/bump-callers/bump-callers.sh index bd86b06..ea7240b 100755 --- a/.github/bump-callers/bump-callers.sh +++ b/.github/bump-callers/bump-callers.sh @@ -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 @@ -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 @@ -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 @@ -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." @@ -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 @@ -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 @@ -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 diff --git a/.github/bump-callers/tests/test_bump_callers.sh b/.github/bump-callers/tests/test_bump_callers.sh index 0bf37fe..eabdf8a 100755 --- a/.github/bump-callers/tests/test_bump_callers.sh +++ b/.github/bump-callers/tests/test_bump_callers.sh @@ -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"