[Changelog] Parameterize nightly compile over configurable branches

[hujc7]

1. Summary

• Parameterize nightly-changelog.yml to compile multiple branches per cron tick, with a single-branch manual override.
• Cron set lives in one place at the top of the file (env.CRON_BRANCHES). Manual trigger takes a required free-text branch input — scales to new release branches without workflow edits.

2. Usage

Cron (5 AM UTC nightly): compiles every branch in env.CRON_BRANCHES. Today: develop,release/3.0.0-beta2.

Manual (single branch, any ref carrying tools/changelog/cli.py):

gh workflow run nightly-changelog.yml \
  -f branch=release/3.0.0-beta2

gh workflow run nightly-changelog.yml \
  -f branch=release/3.0.0-beta2 \
  -f dry_run=true

3. Extending the nightly set

Edit one line:

env:
  CRON_BRANCHES: develop,release/3.0.0-beta2,release/3.0.0

The branch must carry tools/changelog/cli.py and the isaaclab-bot App must be in its branch-ruleset bypass list.

4. Mechanics

• resolve-branches job: emits a JSON array. On schedule events it splits env.CRON_BRANCHES (CSV); on workflow_dispatch events it emits a 1-element array from inputs.branch.
• compile-changelog job: matrix over the array, fail-fast: false, per-branch concurrency. Each entry checks out its target branch, runs that branch's tools/changelog/cli.py, commits, rebases, and pushes back.
• Pattern mirrors daily-compatibility.yml's setup-versions → fromJson matrix.

5. Out of scope / follow-ups

• Stale copies of nightly-changelog.yml on develop and release/3.0.0-beta2 are dead code (cron only fires from main). Optional cleanup PRs against those branches.

[isaaclab-review-bot]

Review Summary

Clean, well-documented workflow refactor that correctly parameterizes the changelog compilation across multiple branches using a matrix strategy.

Strengths

• Consistent pattern: The resolve-branches → matrix approach mirrors daily-compatibility.yml, maintaining codebase consistency
• Proper concurrency handling: Per-branch concurrency groups (nightly-changelog-${{ matrix.branch }}) allow parallel execution across different branches while preventing race conditions on the same branch
• Correct fail-fast behavior: fail-fast: false ensures one branch's failure doesn't cancel others
• Excellent documentation: Both the PR description and inline comments thoroughly explain the design rationale

Potential Edge Cases to Consider

1. Empty input handling: If branches input is somehow an empty string or only whitespace/commas, the resulting empty JSON array [] would produce no matrix jobs. The workflow would succeed with zero work done—a silent no-op. Consider adding a guard:

   if [ "$arr" = "[]" ]; then
     echo "::error::No valid branches resolved"
     exit 1
   fi

2. Branch name validation: Invalid branch names fail at checkout rather than early. For better UX, you could validate branches exist using git ls-remote in the resolver job (though this adds latency and may be overkill).

3. Bypass list documentation: The comment correctly notes each branch needs the App in its bypass list. Consider linking to internal docs or the setup guide if one exists, to help future maintainers adding new branches.

Minor Observations

• The CSV parsing (tr ',\n\ | xargs -n1) handles standard cases well. Branch names with unusual characters (spaces, special chars) could break parsing, but such names are unlikely in practice.
• The dry_run parameter is great for verification—recommend running a dry-run against a release branch before merging to validate the matrix expansion.

Verdict

Looks good overall. The architecture is sound and follows established patterns. The edge cases noted above are low-probability but worth considering for robustness.

---

Automated review by isaaclab-review-bot

---

Update (43045b4): Reviewed new commit. Changes are documentation-only—improved header comments explaining multi-branch behavior and how to extend the branch set. No functional changes. Previous suggestions remain optional improvements; no new issues introduced. ✅ LGTM

---

Update (e2e90d3): Continued header comment refinements—clarifies bypass-list requirement per target branch and explains the single-input manual trigger design. No functional changes. ✅ LGTM

---

Update (f0cd598): Reviewed latest commit. Notable security improvement: now uses TARGET_BRANCH env var passthrough instead of direct ${{ matrix.branch }} interpolation in shell scripts—correctly prevents potential shell injection from adversarial branch names. Added refs/heads/ prefix for explicit ref disambiguation. Implementation is solid. ✅ LGTM

---

Update (9d49286): ⚠️ New issue introduced. The resolve-branches step now has duplicate env: keys (lines ~75-78):

- id: b
  env:
    BRANCHES: ${{ ... }}
  env:              # ← Duplicate key!
    EVENT_NAME: ${{ github.event_name }}

In YAML, duplicate keys in the same mapping cause the second to silently override the first. This means BRANCHES won't be set, breaking the workflow. The fix is to combine both variables into a single env: block:

- id: b
  env:
    BRANCHES: ${{ github.event_name == 'schedule' && env.CRON_BRANCHES || inputs.branch }}
    EVENT_NAME: ${{ github.event_name }}
  run: |
    ...

This must be fixed before merging.

[greptile-apps]

Greptile Summary

This PR refactors nightly-changelog.yml from a single hardcoded develop branch into a matrix-driven pipeline that compiles changelogs for every branch in a top-level env.CRON_BRANCHES CSV, while a workflow_dispatch path accepts a free-text branch input for one-off runs.

• A new resolve-branches job converts the CSV (or the manual input) to a JSON array; compile-changelog fans out over it with fail-fast: false and per-branch concurrency groups, mirroring the pattern in daily-compatibility.yml.
• Dynamic ref: ${{ matrix.branch }}, git pull --rebase, and git push replace the previously hardcoded develop references throughout the checkout and push steps.
• Three quality issues were found: ${{ matrix.branch }} and other expressions are interpolated directly into shell run blocks rather than passed through environment variables (the standard hardening for GitHub Actions); a comment on line 16 references a non-existent branches: input instead of env.CRON_BRANCHES; and a comma in the manual branch input silently fans out to multiple branches, contradicting the documented single-branch intent.

Confidence Score: 4/5

Safe to merge for well-formed branch names; no correctness or data-loss risk exists in normal nightly operation.

The matrix fan-out logic, per-branch concurrency, and dynamic checkout/push mechanics are all sound. The only concerns are direct expression interpolation in shell run blocks (exploitable only by write-access actors with carefully crafted branch names) and two minor documentation issues.

.github/workflows/nightly-changelog.yml — specifically the run: blocks that use ${{ matrix.branch }} and ${{ inputs.dry_run }} as inline text rather than via environment variables.

Security Review

• Expression injection in shell run steps (.github/workflows/nightly-changelog.yml, lines 203–206): ${{ matrix.branch }} is text-substituted into the shell command body before the shell executes it. On the workflow_dispatch path, this value comes directly from user-provided free-text input. A branch string containing " and shell metacharacters could break out of the quoted argument and run arbitrary commands in the runner. The risk is bounded to actors with repository write access (required to trigger workflow_dispatch), but the pattern is exploitable and should be replaced with environment-variable indirection. The same pattern appears at lines 154 and 188 for inputs.dry_run and github.event_name.

Important Files Changed

Filename

Overview

.github/workflows/nightly-changelog.yml

Refactored from single-branch hardcoded workflow to a matrix-driven multi-branch pipeline; introduces a resolve-branches job, per-branch concurrency groups, and a workflow_dispatch branch input. Three P2 findings: direct expression interpolation in shell commands, a stale comment referencing a non-existent branches: input, and undocumented multi-branch behavior when a user enters a comma in the manual-trigger field.

Sequence Diagram

sequenceDiagram
    participant T as Trigger (cron / dispatch)
    participant RB as resolve-branches job
    participant CC1 as compile-changelog (develop)
    participant CC2 as compile-changelog (release/3.0.0-beta2)
    participant GH as GitHub (target branch)

    T->>RB: event_name + CRON_BRANCHES or inputs.branch
    RB->>RB: CSV to JSON array via tr/xargs/jq
    RB-->>CC1: "branches[0] = develop"
    RB-->>CC2: "branches[1] = release/3.0.0-beta2"

    par matrix fan-out (fail-fast: false)
        CC1->>GH: create-github-app-token
        GH-->>CC1: short-lived token
        CC1->>GH: "checkout ref=develop"
        CC1->>CC1: python3 tools/changelog/cli.py compile --all
        CC1->>GH: git pull --rebase + git push HEAD:develop

        CC2->>GH: create-github-app-token
        GH-->>CC2: short-lived token
        CC2->>GH: "checkout ref=release/3.0.0-beta2"
        CC2->>CC2: python3 tools/changelog/cli.py compile --all
        CC2->>GH: git pull --rebase + git push HEAD:release/3.0.0-beta2
    end

Loading

_{Reviews (1): Last reviewed commit: "[Changelog] Parameterize nightly compile..." | Re-trigger Greptile}

[greptile-apps]

Direct expression interpolation in shell scripts

${{ matrix.branch }} is text-substituted into the shell command before the shell runs it. For the workflow_dispatch path, inputs.branch is free-text, so a value containing " followed by shell metacharacters (e.g., develop"; id; #) would break out of the quoted string and execute arbitrary commands. Although this is limited to actors with repository write access (needed to trigger workflow_dispatch), the standard hardening is to pass the value through an environment variable so the shell never sees it as part of the command text. The same pattern applies to the ${{ inputs.dry_run }} interpolation at line 154 and ${{ github.event_name }} at line 188.

[greptile-apps]

Stale reference in comment

The comment says "one-line edit to the branches: input default below", but the workflow has no branches: input. The single source of truth is env.CRON_BRANCHES on line 39. The comment should reference that variable to avoid confusion for future maintainers.

[greptile-apps]

Comma in inputs.branch silently fans out to multiple branches on a manual run

The PR description says "Manual trigger is always a single branch", and the branch input description reinforces this, but the resolve-branches step applies the same CSV-split-and-trim logic regardless of event type. A maintainer who types develop,release/3.0.0-beta2 in the dispatch form gets a two-branch matrix run instead of a clear error. If the intent is strictly one branch for manual runs, adding a guard — e.g., checking that the resolved array length is 1 when github.event_name == 'workflow_dispatch' — would surface the mistake early rather than silently expanding the run.