Conversation
BlackYps
added
the
area: changelog management
Contributor
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow step that creates/updates changelog snippet files for PRs, introduces two Jekyll plugins (a post-render balance-change transformer and a Changes
Sequence DiagramsequenceDiagram
participant GH as GitHub
participant GHA as GitHub Actions
participant Git as Git
participant FS as FileSystem
participant CLI as gh CLI
GH->>GHA: PR opened (workflow trigger)
GHA->>Git: checkout PR branch
GHA->>FS: copy template-snippet.md -> changelog/snippets/category.<PR_NUMBER>.md
GHA->>FS: replace "XYZW" with PR number
GHA->>Git: git add, commit, push
GHA->>CLI: run 'gh pr ready --undo'
CLI->>GH: request to mark PR draft
GH-->>CLI: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
docs/Gemfile (1)
9-11: Avoid pinning default Ruby gems unless runtime is explicitly pinned.
csvandbase64are default gems tied closely to Ruby runtime versions. Pinning them here without an explicit Ruby version for the docs toolchain can cause Bundler resolution mismatches across local/CI environments. Prefer relying on default gems, or add a strictrubyversion and validate lockfile consistency.#!/bin/bash # Read-only verification for Ruby/default-gem pinning risk in docs toolchain. set -euo pipefail echo "== Find Ruby version constraints ==" rg -n --iglob 'Gemfile*' --iglob '*.gemspec' --iglob '.ruby-version' '\bruby\b|^ruby-version|^RUBY_VERSION' echo echo "== Inspect docs Gemfile/Gemfile.lock entries for csv/base64 ==" fd -i 'Gemfile.lock|Gemfile' docs . | xargs -r rg -n '^\s{0,4}(csv|base64)\b|gem\s+"(csv|base64)"' echo echo "== Locate actual requires/usages in docs-related Ruby code ==" rg -n --type ruby -C2 'require\s+["'\''](csv|base64)["'\'']|CSV\b|Base64\b' docs🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/Gemfile` around lines 9 - 11, The Gemfile currently pins default Ruby stdlib gems with lines gem "csv", "~> 3.3" and gem "base64", "~> 0.3.0", which can cause Bundler resolution mismatches; either remove these gem entries so the toolchain uses the runtime-provided stdlib versions, or instead add an explicit ruby version constraint (ruby "x.y.z") and update/commit a matching Gemfile.lock so the pinned versions are intentional and reproducible—change the Gemfile by deleting the gem "csv" and gem "base64" lines or add a strict ruby declaration and regenerate the lockfile, referencing the existing Gemfile entries (gem "csv"/gem "base64") to locate the edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-draft-pr.yaml:
- Around line 34-47: The workflow step that creates and commits a snippet is
missing a repository checkout, a push of the new commit, and uses an
unrecognized filename pattern; to fix it add an initial "actions/checkout@vX"
step before changing into fa/changelog/snippets, update FILE from category.${{
github.event.pull_request.number }}.md to one of the accepted patterns (e.g.
other.${{ github.event.pull_request.number }}.md or features.${{
github.event.pull_request.number }}.md) so the bundler will pick it up, and
after git commit run git push to push the new branch/commit; ensure the existing
commands that reference FILE, cp sections/template-snippet.md, sed -i
"s/XYZW/${{ github.event.pull_request.number }}/g" $FILE, git add ., and git
commit -m "Add snippet template" remain but run only after checkout and before
the git push.
In `@docs/_plugins/balance_change.rb`:
- Around line 8-10: The current regex in doc.output.gsub! (the
/([^<\s]+)\s*-->\s*([^<\s]+)/) only captures single tokens (no spaces or
parentheses), so multi-token values like "60 (225) --> 45 (225)" are missed;
update the pattern to allow any non-HTML content around the arrow (e.g. use lazy
captures like /([^<]+?)\s*-->\s*([^<]+?)/ or similar) and trim captured
groups before inserting them into the replacement span so leading/trailing
whitespace isn't preserved; keep the replacement logic in the same gsub! call
but switch to the broader capture groups (referencing doc.output.gsub! and the
regex literal) and ensure you still escape HTML-sensitive chars if needed.
In `@docs/_plugins/unit_block.rb`:
- Around line 13-16: The template injects raw `@unit_id` and name into HTML;
update the UnitBlock rendering to validate `@unit_id` against a strict
whitelist/regex (e.g., only [A-Za-z0-9_-]) and reject or normalize unexpected
values, and escape any user-provided text using an HTML-escaping helper (e.g.,
CGI.escapeHTML or Jekyll::Utils.escape_html) when outputting name and when
composing the img src attribute; locate the renderer method that outputs the div
with class "unit-header" (the code using `@unit_id` and name) and apply these
validations/escapes so the src="/assets/icons/#{`@unit_id`}.png" and the span
content use safe, escaped values.
In `@docs/development-changelog/balance-snippet.md`:
- Line 19: Add a fenced language specifier to the code block starting at the
opening ``` on line 19 by changing it to ```md, and fix the heading level by
replacing the "### Example snippets" heading with "## Example snippets" (update
the heading text referenced as "### Example snippets" to "## Example snippets")
so markdownlint no longer reports a missing fence language and a skipped heading
level.
- Around line 9-14: Remove the draft/internal notes lines in the balance-snippet
content (the brainstorming lines about "Unit or explanation first?", "The unitID
is also used...", etc.) so the page reads as polished guidance; also ensure the
md->lua converter strips the inline marker "{: .unit-change}" and removes the
trailing "(unitID)" tokens (including cases like "enhancements/faction/name")
from rendered output so no draft markers or raw unit IDs remain in published
docs.
In `@docs/development-changelog/other-snippet.md`:
- Around line 12-13: The markdown violates lint rules by skipping a heading
level and omitting a fence language: change the "### Example snippet" heading to
"## Example snippet" and add the language tag "md" to the opening code fence for
the example (the fenced block that contains the LOD formula line) so it reads as
a proper fenced code block with a declared language.
---
Nitpick comments:
In `@docs/Gemfile`:
- Around line 9-11: The Gemfile currently pins default Ruby stdlib gems with
lines gem "csv", "~> 3.3" and gem "base64", "~> 0.3.0", which can cause Bundler
resolution mismatches; either remove these gem entries so the toolchain uses the
runtime-provided stdlib versions, or instead add an explicit ruby version
constraint (ruby "x.y.z") and update/commit a matching Gemfile.lock so the
pinned versions are intentional and reproducible—change the Gemfile by deleting
the gem "csv" and gem "base64" lines or add a strict ruby declaration and
regenerate the lockfile, referencing the existing Gemfile entries (gem "csv"/gem
"base64") to locate the edits.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 700c6765-d17f-4090-9a14-9a88974a38aa
⛔ Files ignored due to path filters (2)
docs/Gemfile.lockis excluded by!**/*.lockdocs/assets/icons/XES0102.pngis excluded by!**/*.png
📒 Files selected for processing (10)
.github/workflows/auto-draft-pr.yamlchangelog/snippets/sections/template-snippet.mddocs/Gemfiledocs/_plugins/balance_change.rbdocs/_plugins/unit_block.rbdocs/_sass/custom/custom.scssdocs/development-changelog.mddocs/development-changelog/balance-snippet.mddocs/development-changelog/other-snippet.mddocs/documentation.md
Comment on lines
+13
to
+16
| <div class="unit-header" data-unit="#{@unit_id}"> | ||
| <img class="unit-icon" | ||
| src="/assets/icons/#{@unit_id}.png"> | ||
| <span class="unit-name">#{name}</span> |
Contributor
There was a problem hiding this comment.
Escape and validate interpolated HTML values.
@unit_id and name are injected raw into HTML. This should be escaped (and unit_id validated) before rendering.
🛡️ Suggested patch
+require "cgi"
+
module Jekyll
class UnitBlock < Liquid::Block
@@
def render(context)
- name = super.strip
+ name = CGI.escapeHTML(super.strip)
+ unit_id = `@unit_id.strip`
+ raise ArgumentError, "Invalid unit id" unless unit_id.match?(/\A[a-zA-Z0-9_\/-]+\z/)
+ safe_unit_id = CGI.escapeHTML(unit_id)
<<~HTML
- <div class="unit-header" data-unit="#{`@unit_id`}">
+ <div class="unit-header" data-unit="#{safe_unit_id}">
<img class="unit-icon"
- src="/assets/icons/#{`@unit_id`}.png">
+ src="/assets/icons/#{safe_unit_id}.png"
+ alt="">
<span class="unit-name">#{name}</span>
</div>
HTML🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/_plugins/unit_block.rb` around lines 13 - 16, The template injects raw
`@unit_id` and name into HTML; update the UnitBlock rendering to validate `@unit_id`
against a strict whitelist/regex (e.g., only [A-Za-z0-9_-]) and reject or
normalize unexpected values, and escape any user-provided text using an
HTML-escaping helper (e.g., CGI.escapeHTML or Jekyll::Utils.escape_html) when
outputting name and when composing the img src attribute; locate the renderer
method that outputs the div with class "unit-header" (the code using `@unit_id`
and name) and apply these validations/escapes so the
src="/assets/icons/#{`@unit_id`}.png" and the span content use safe, escaped
values.
Member
I think it may look inconsistent visually when you put it last though. The idea of it being first was originally to emphasize the development that happens behind the scenes, to lure in possible contributors with knowledge of GitHub to know how to look it up. This is also why these numbers are made clickable by I think @relent0r at the time: 
This appears to be used actively, see also the 
The highlighted is likely a reference to https://faforever.github.io/fa/. I did eventually stop putting pull request or issue numbers behind the names of contributors, as was a little skewed and this new flow would not support that either way. I do very much like the richer visuals for balance changes 😄 ! |
Contributor
Author
|
I'm not arguing to get rid of the PR numbers or the links for that matter. But I don't think it is critical for luring in contributors that the link comes first. If we think that we should encourage new people more I would rather do it by adding something to the release text. |
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
docs/_plugins/unit_block.rb (1)
10-16:⚠️ Potential issue | 🟠 MajorSanitize interpolated values before rendering HTML
Line 13–16 inject
@unit_idandnameraw into HTML. This is still vulnerable to markup/attribute injection and malformedsrcpaths. Validate@unit_idagainst a strict allowlist and HTML-escape both fields before interpolation.🔧 Proposed fix
+require "cgi" + module Jekyll class UnitBlock < Liquid::Block @@ def render(context) - name = super.strip + name = CGI.escapeHTML(super.strip) + unit_id = `@unit_id.to_s.strip` + raise ArgumentError, "Invalid unit id" unless unit_id.match?(/\A[A-Za-z0-9_-]+\z/) + safe_unit_id = CGI.escapeHTML(unit_id) <<~HTML - <div class="unit-header" data-unit="#{`@unit_id`}"> + <div class="unit-header" data-unit="#{safe_unit_id}"> <img class="unit-icon" - src="/assets/icons/#{`@unit_id`}.png"> + src="/assets/icons/#{safe_unit_id}.png" + alt="#{name} icon"> <span class="unit-name">#{name}</span> </div> HTML end end end🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/_plugins/unit_block.rb` around lines 10 - 16, Sanitize and validate the interpolated values before rendering: validate `@unit_id` against a strict allowlist (reject or normalize any value not in the allowlist) and HTML-escape both `@unit_id` and the `name` returned by super.strip prior to interpolation in the unit header template (the code constructing the string with `<div class="unit-header" data-unit="#{`@unit_id`}">` and the `<img src="/assets/icons/#{`@unit_id`}.png">` and `<span class="unit-name">#{name}</span>`); ensure the sanitized/escaped unit id is used to build the src path (or provide a safe default/fallback image) and that `name` is escaped to prevent markup injection.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/_plugins/unit_block.rb`:
- Around line 10-16: Sanitize and validate the interpolated values before
rendering: validate `@unit_id` against a strict allowlist (reject or normalize any
value not in the allowlist) and HTML-escape both `@unit_id` and the `name`
returned by super.strip prior to interpolation in the unit header template (the
code constructing the string with `<div class="unit-header"
data-unit="#{`@unit_id`}">` and the `<img src="/assets/icons/#{`@unit_id`}.png">`
and `<span class="unit-name">#{name}</span>`); ensure the sanitized/escaped unit
id is used to build the src path (or provide a safe default/fallback image) and
that `name` is escaped to prevent markup injection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 962fa51a-cf50-47f2-a62a-0ecaedc57f79
⛔ Files ignored due to path filters (113)
docs/assets/icons/DAA0206_icon.pngis excluded by!**/*.pngdocs/assets/icons/DAB2102_icon.pngis excluded by!**/*.pngdocs/assets/icons/DAL0310_icon.pngis excluded by!**/*.pngdocs/assets/icons/DEA0202_icon.pngis excluded by!**/*.pngdocs/assets/icons/DEL0204_icon.pngis excluded by!**/*.pngdocs/assets/icons/DRA0202_icon.pngis excluded by!**/*.pngdocs/assets/icons/DRL0204_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0102_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0104_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0107_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0203_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0204_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0303_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0304_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAA0310_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0102_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0201_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0203_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0301_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0303_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB0304_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1101_over.pngis excluded by!**/*.pngdocs/assets/icons/UAB1102_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1104_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1105_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1106_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1201_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1301_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1303_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2104_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2108_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2109_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2204_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2205_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2301_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2303_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2304_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB2305_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB3101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB3102_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB3104_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB3201_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB3202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB4201_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB4202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB4203_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB4301_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB4302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB5101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB5103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB5202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0001_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0104_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0105_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0106_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0111_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0201_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0205_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0208_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0301_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0303_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0304_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0307_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0309_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAL0401_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0102_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0201_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0203_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0303_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0304_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0305_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAS0401_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0001_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0003_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0102_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0104_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0107_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0203_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0204_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0303_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0304_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEA0305_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0101_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0102_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0103_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0201_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0202_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0203_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0301_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0302_icon.pngis excluded by!**/*.pngdocs/assets/icons/UEB0303_icon.pngis excluded by!**/*.png
📒 Files selected for processing (5)
.github/workflows/auto-draft-pr.yamlchangelog/snippets/sections/template-snippet.mddocs/_plugins/balance_change.rbdocs/_plugins/unit_block.rbdocs/_sass/custom/custom.scss
✅ Files skipped from review due to trivial changes (3)
- changelog/snippets/sections/template-snippet.md
- docs/_sass/custom/custom.scss
- .github/workflows/auto-draft-pr.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/_plugins/balance_change.rb
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/auto-draft-pr.yaml:
- Around line 30-33: The prepare_pr job can fail on forked PRs because it uses a
repo secret and pushes commits; update the prepare_pr job to run only for
same-repo pull requests by adding a conditional that checks the PR head
repository matches the target repository (e.g., compare
github.event.pull_request.head.repo.full_name to github.repository or
github.event.pull_request.head.repo.owner.login to github.repository_owner) so
the job is skipped for forks, and ensure any steps that use the secret or
perform git push remain inside that guarded job (referencing the prepare_pr job
name).
- Around line 35-39: The checkout step using actions/checkout@v6 must check out
the PR head branch instead of the detached merge ref and the later git push must
target that branch explicitly; update the checkout step to use ref: ${{
github.head_ref }} (and ensure fetch-depth is sufficient) so the workflow is on
the PR branch, and change the push invocation referenced after that step to push
to the same branch explicitly (e.g., push origin HEAD:refs/heads/${{
github.head_ref }} or use --set-upstream origin ${{ github.head_ref }}), keeping
the existing ssh-key input intact.
In @.github/workflows/scripts/python/changelog_markdown2lua.py:
- Around line 102-105: The current remove_liquid_tags function's regex
r"{%.*?%}\s*\n" misses tags that span multiple lines or appear at EOF; update
the pattern to match across newlines and allow no trailing newline (e.g., use a
non-greedy "{%.*?%}" with re.DOTALL and replace trailing whitespace/newline
optionally) by calling re.sub with flags=re.DOTALL or change the pattern to
r"{%.*?%}\s*(?:\n|$)" so multiline Liquid tags and tags at EOF are removed by
remove_liquid_tags.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0c26c370-7ac6-4de0-a983-76fbfd324438
⛔ Files ignored due to path filters (5)
docs/assets/icons/DALK003_icon.pngis excluded by!**/*.pngdocs/assets/icons/DELK002_icon.pngis excluded by!**/*.pngdocs/assets/icons/DRLK001_icon.pngis excluded by!**/*.pngdocs/assets/icons/DSLK004_icon.pngis excluded by!**/*.pngdocs/assets/icons/UAB1101_icon.pngis excluded by!**/*.png
📒 Files selected for processing (2)
.github/workflows/auto-draft-pr.yaml.github/workflows/scripts/python/changelog_markdown2lua.py
Comment on lines
30
to
33
| jobs: | ||
| convert_to_draft: | ||
| prepare_pr: | ||
| name: Prepare pull request | ||
| runs-on: ubuntu-latest |
Contributor
There was a problem hiding this comment.
Guard this job for same-repo PRs to avoid fork failures.
Line 38 uses a repo secret and Line 53 pushes commits; forked PRs typically cannot use that secret or accept this push path.
Suggested fix
jobs:
prepare_pr:
name: Prepare pull request
+ if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}
runs-on: ubuntu-latestAlso applies to: 35-39, 53-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/auto-draft-pr.yaml around lines 30 - 33, The prepare_pr
job can fail on forked PRs because it uses a repo secret and pushes commits;
update the prepare_pr job to run only for same-repo pull requests by adding a
conditional that checks the PR head repository matches the target repository
(e.g., compare github.event.pull_request.head.repo.full_name to
github.repository or github.event.pull_request.head.repo.owner.login to
github.repository_owner) so the job is skipped for forks, and ensure any steps
that use the secret or perform git push remain inside that guarded job
(referencing the prepare_pr job name).
Comment on lines
+35
to
+39
| - name: Checkout branch | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| ssh-key: ${{ secrets.SSH_FAFOREVER_MACHINE_USER }} | ||
|
|
Contributor
There was a problem hiding this comment.
Checkout must target the PR head branch before pushing.
On pull_request, checkout defaults to a detached merge ref; then Line 53 git push has no explicit branch target and can fail.
Suggested fix
- - name: Checkout branch
+ - name: Checkout PR head branch
uses: actions/checkout@v6
with:
+ ref: ${{ github.event.pull_request.head.ref }}
+ repository: ${{ github.event.pull_request.head.repo.full_name }}
ssh-key: ${{ secrets.SSH_FAFOREVER_MACHINE_USER }}
...
- git push
+ git push origin "HEAD:${{ github.event.pull_request.head.ref }}"Also applies to: 53-53
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/auto-draft-pr.yaml around lines 35 - 39, The checkout step
using actions/checkout@v6 must check out the PR head branch instead of the
detached merge ref and the later git push must target that branch explicitly;
update the checkout step to use ref: ${{ github.head_ref }} (and ensure
fetch-depth is sufficient) so the workflow is on the PR branch, and change the
push invocation referenced after that step to push to the same branch explicitly
(e.g., push origin HEAD:refs/heads/${{ github.head_ref }} or use --set-upstream
origin ${{ github.head_ref }}), keeping the existing ssh-key input intact.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the proposed changes
We should reconsider the snippet formats a bit. I think it makes more sense when the PR number comes last. It is not that important that it needs to come first.
Additionally. we can bump the quality of the changelogs on the website a lot by adding some processing. This requires a little additional syntax in the balance snippets, but it's not too complicated.
Yanky first results:
Testing done on the proposed changes
Additional context
Checklist
Summary by CodeRabbit
New Features
Chores