Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to fix the changelog generation GitHub Actions workflow by adjusting the inputs passed to the actions/create-github-app-token steps so the automation can successfully obtain GitHub App tokens and create/update changelog PRs.
Changes:
- Updates the token-generation steps to pass
client-idinstead ofapp-idfor both the read-token and write-token GitHub Apps.
📝 WalkthroughWalkthroughNormalize PR body line endings and require a ChangesChangelog generation DAG
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/generate-changelog.yml (1)
79-79:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHardcoded
CHANGELOG_PATHwill silently write future changelogs into the wrong file.
source/product-overview/mattermost-v11-changelog.mdis fixed regardless of theversioninput. Running the workflow for v12+ will append content to the v11 file instead of the appropriate one.🛠️ Suggested fix — derive the path from the `version` input
- CHANGELOG_PATH: source/product-overview/mattermost-v11-changelog.md + CHANGELOG_PATH: source/product-overview/mattermost-v${{ inputs.version }}-changelog.mdNote: this assumes the changelog filename encodes the minor version only (e.g.
10.6→mattermost-v10.6-changelog.md). Adjust the expression if the major-version grouping (v11) is intentional and a separate mapping is needed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/generate-changelog.yml at line 79, The hardcoded CHANGELOG_PATH will always point to source/product-overview/mattermost-v11-changelog.md; update the workflow to compute CHANGELOG_PATH from the workflow input (the version input) instead of a fixed string — read the version input (e.g., inputs.version or github.event.inputs.version) and build the filename using the expected pattern (e.g., mattermost-v{minor-major?}-changelog.md or mattermost-v{version}-changelog.md depending on your naming convention), then set CHANGELOG_PATH to that constructed value so running the workflow for v12+ writes to the correct file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/generate-changelog.yml:
- Line 79: The hardcoded CHANGELOG_PATH will always point to
source/product-overview/mattermost-v11-changelog.md; update the workflow to
compute CHANGELOG_PATH from the workflow input (the version input) instead of a
fixed string — read the version input (e.g., inputs.version or
github.event.inputs.version) and build the filename using the expected pattern
(e.g., mattermost-v{minor-major?}-changelog.md or
mattermost-v{version}-changelog.md depending on your naming convention), then
set CHANGELOG_PATH to that constructed value so running the workflow for v12+
writes to the correct file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 388db96a-f75a-4f65-b56b-60de8c7313ed
📒 Files selected for processing (1)
.github/workflows/generate-changelog.yml
|
Newest code from mattermost has been published to preview environment for Git SHA d8218f0 |
|
Newest code from mattermost has been published to preview environment for Git SHA 8d33775 |
|
Newest code from mattermost has been published to preview environment for Git SHA 16ed2e1 |
|
Newest code from mattermost has been published to preview environment for Git SHA 3d8cad2 |
|
Newest code from mattermost has been published to preview environment for Git SHA c911c96 |
|
Re-requesting a review as I added a few other minor fixes. |
|
Newest code from mattermost has been published to preview environment for Git SHA c703e1d |
|
Newest code from mattermost has been published to preview environment for Git SHA 6fd3820 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/generate-changelog.yml (1)
87-103:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
gh pr createwill fail on workflow re-runs if a PR already exists for the branch.If the workflow is re-triggered for the same version (e.g., to regenerate after a script fix),
git push --forcewill succeed butgh pr createwill exit non-zero because the PR already exists, failing the step.🛡️ Proposed fix
- gh pr create \ - --title "${{ inputs.version }} Changelog" \ - --body "Auto-generated changelog for milestone \`${{ inputs.milestone }}\` from \`${{ inputs.server_repos }}\`." \ - --base master \ - --head "changelog/${{ inputs.version }}" + gh pr list --head "changelog/${{ inputs.version }}" --json number --jq '.[0].number' | grep -q . || \ + gh pr create \ + --title "${{ inputs.version }} Changelog" \ + --body "Auto-generated changelog for milestone \`${{ inputs.milestone }}\` from \`${{ inputs.server_repos }}\`." \ + --base master \ + --head "changelog/${{ inputs.version }}"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/generate-changelog.yml around lines 87 - 103, The gh pr create step fails if a PR already exists for the "changelog/${{ inputs.version }}" branch; update the workflow to check for an existing PR before calling gh pr create (e.g., run gh pr list or gh api to search for a PR with head "changelog/${{ inputs.version }}" and only run gh pr create when none is found), and if a PR exists, echo a message and skip creation so the step exits successfully; ensure you reference the branch name "changelog/${{ inputs.version }}" and the gh pr create invocation when implementing the conditional check.
🧹 Nitpick comments (2)
.github/scripts/generate_changelog.py (2)
262-268: ⚡ Quick win
BLOG_POST_URLplaceholder replacement could silently fail if Claude reformats it.
polished.replace("BLOG_POST_URL", blog_url)is a bare string replace with no verification that the substitution actually occurred. If the model rephrases the sentence or places the placeholder inside angle brackets or quotes (deviating from the system prompt instruction), the literal stringBLOG_POST_URLwill appear in the published changelog.Consider asserting the replacement happened, or logging a warning:
🛡️ Proposed guard
polished = polished.replace("BLOG_POST_URL", blog_url) + if "BLOG_POST_URL" in polished: + print("⚠️ Warning: BLOG_POST_URL placeholder was not replaced — check AI output") entry += polished + "\n"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/generate_changelog.py around lines 262 - 268, Ensure the placeholder substitution for BLOG_POST_URL actually took place: after computing blog_url (using VERSION) and performing polished = polished.replace("BLOG_POST_URL", blog_url), verify that the polished string no longer contains the placeholder (or variants like "<BLOG_POST_URL>" or quoted forms) and fail fast or emit a warning if it remains; update the code around the polished.replace call to check polished for the original token(s) and log an error or raise an exception (using process logger or similar) including the offending polished snippet so the replacement issue is visible and prevents publishing.
246-258: ⚡ Quick winThe "Extended Support Release" label and anchor are unconditional — non-ESR versions will be mislabelled.
The heading and anchor always include
-extended-support-releaseregardless of theVERSIONinput. If the script is ever run for a regular (non-ESR) milestone, the generated changelog heading will be incorrect.Consider adding an optional
is_esrworkflow input and toggling the heading/anchor format accordingly:♻️ Proposed change
- # e.g. (release-v11.7-extended-support-release)= - anchor = f"(release-{version_short}-extended-support-release)=" - heading = ( - f"## Release {version_short} - " - f"[Extended Support Release]" - f"(https://docs.mattermost.com/product-overview/release-policy.html#release-types)" - ) - entry = f"{anchor}\n{heading}\n\n**Release day: {release_date}**\n\n" + is_esr = os.environ.get("IS_ESR", "false").strip().lower() in ("1", "true", "yes") + if is_esr: + anchor = f"(release-{version_short}-extended-support-release)=" + heading = ( + f"## Release {version_short} - " + f"[Extended Support Release]" + f"(https://docs.mattermost.com/product-overview/release-policy.html#release-types)" + ) + else: + anchor = f"(release-{version_short})=" + heading = f"## Release {version_short}" + entry = f"{anchor}\n{heading}\n\n**Release day: {release_date}**\n\n"And add the corresponding input to the workflow:
is_esr: description: 'Whether this is an Extended Support Release' required: false default: 'false' type: boolean🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/generate_changelog.py around lines 246 - 258, The anchor/heading generation always appends "-extended-support-release" regardless of release type; update the logic to accept an is_esr flag (passed from workflow) and conditionally build anchor and heading: keep existing VERSION/version_short and release_date usage, but when is_esr is true set anchor to f"(release-{version_short}-extended-support-release)=" and heading to the current ESR string, otherwise set anchor to f"(release-{version_short})=" and heading to a non-ESR string (e.g., f"## Release {version_short} - [Release]" or similar); ensure entry uses the selected anchor and heading variables and that is_esr defaults to false when not provided.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/scripts/generate_changelog.py:
- Line 267: The print statement that logs the auto-constructed blog URL uses the
ambiguous Unicode character "ℹ" and should be replaced with either plain ASCII
"i" or the consistent emoji sequence "ℹ️" used elsewhere; update the print call
that references blog_url (the line: print(f"ℹ️ No blog post URL provided —
using auto-constructed URL: {blog_url}")) to use the standard "ℹ️" emoji
sequence or "i" to satisfy Ruff RUF001, keeping the rest of the message and the
blog_url interpolation unchanged.
---
Outside diff comments:
In @.github/workflows/generate-changelog.yml:
- Around line 87-103: The gh pr create step fails if a PR already exists for the
"changelog/${{ inputs.version }}" branch; update the workflow to check for an
existing PR before calling gh pr create (e.g., run gh pr list or gh api to
search for a PR with head "changelog/${{ inputs.version }}" and only run gh pr
create when none is found), and if a PR exists, echo a message and skip creation
so the step exits successfully; ensure you reference the branch name
"changelog/${{ inputs.version }}" and the gh pr create invocation when
implementing the conditional check.
---
Nitpick comments:
In @.github/scripts/generate_changelog.py:
- Around line 262-268: Ensure the placeholder substitution for BLOG_POST_URL
actually took place: after computing blog_url (using VERSION) and performing
polished = polished.replace("BLOG_POST_URL", blog_url), verify that the polished
string no longer contains the placeholder (or variants like "<BLOG_POST_URL>" or
quoted forms) and fail fast or emit a warning if it remains; update the code
around the polished.replace call to check polished for the original token(s) and
log an error or raise an exception (using process logger or similar) including
the offending polished snippet so the replacement issue is visible and prevents
publishing.
- Around line 246-258: The anchor/heading generation always appends
"-extended-support-release" regardless of release type; update the logic to
accept an is_esr flag (passed from workflow) and conditionally build anchor and
heading: keep existing VERSION/version_short and release_date usage, but when
is_esr is true set anchor to
f"(release-{version_short}-extended-support-release)=" and heading to the
current ESR string, otherwise set anchor to f"(release-{version_short})=" and
heading to a non-ESR string (e.g., f"## Release {version_short} - [Release]" or
similar); ensure entry uses the selected anchor and heading variables and that
is_esr defaults to false when not provided.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: da988327-a168-4984-be45-bb47eda4550a
📒 Files selected for processing (2)
.github/scripts/generate_changelog.py.github/workflows/generate-changelog.yml
|
Newest code from mattermost has been published to preview environment for Git SHA 33b398e |
amyblais
added
3: Reviews Complete
|
Newest code from mattermost has been published to preview environment for Git SHA 8a249ed |
amyblais
removed
the
3: Reviews Complete
Quick fix for the changelog automation workflow based on the failure at https://github.com/mattermost/docs/actions/runs/25428124725.
Added a few additional minor fixes.