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