Setup CI to pull the class DB from the engine#1198
Setup CI to pull the class DB from the engine#1198Arctis-Fireblight merged 48 commits intoRedot-Engine:masterfrom
Conversation
…-Engine#4: Triggger based on workflow.
…-Engine#4: Allow manual run.
…-Engine#4: Add condition to run only of merged
…-Engine#4: Target master branch.
Redot-Engine#4: Test." This reverts commit 8b6c3b5.
…-Engine#4: Change Action condition.
Redot-Engine#4: Test." This reverts commit b0770cb.
…-Engine#4: Change condition path.
Redot-Engine#4: Test." This reverts commit 57a6615.
Redot-Engine#4: Test." This reverts commit 8329c54.
Redot-Engine#4: Test." This reverts commit 635a508.
Redot-Engine#4: Test." This reverts commit 0e232ac.
Redot-Engine#4: Test." This reverts commit fb3e6cf.
Redot-Engine#4: Test." This reverts commit 98a49e7.
Sample changes
…-Engine#4: Change trigger condition.
Sample changes
…-Engine#4: Remove merge changes.
…-Engine#4: Change repo name.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 WalkthroughAdds a new GitHub Actions workflow Changes
Sequence Diagram(s)sequenceDiagram
participant Contributor as Contributor
participant SourceAction as Source Repo Actions
participant GitHubAPI as GitHub API
participant ExternalWorkflow as Redot-Engine/redot-docs-site
Contributor->>SourceAction: Open & merge PR touching doc/classes/**
SourceAction->>GitHubAPI: Detect pull_request closed (merged) on master
GitHubAPI->>SourceAction: Deliver event payload
SourceAction->>ExternalWorkflow: Dispatch external workflow (event: "sync_class_ref", secret: github_pat, env: PAYLOAD_AUTHOR)
ExternalWorkflow->>GitHubAPI: Start external workflow run
ExternalWorkflow->>ExternalWorkflow: Run sync_class_ref job
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
.github/workflows/sync_class_ref.yml (1)
10-14: Add apermissionsblock to enforce least-privilegeThe job has no
permissionsdeclaration, so it inherits the repository default (oftenwrite-all). Since this job only dispatches an external event, it needs no repository permissions at all.♻️ Proposed fix
jobs: trigger: name: Trigger update class reference files based on the engine revision + permissions: {} if: github.event.pull_request.merged == true runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync_class_ref.yml around lines 10 - 14, The job named "trigger" currently has no explicit permissions and so inherits repository defaults; add a job-level permissions declaration to enforce least privilege by inserting a permissions: none (or an empty permissions map) under the "trigger" job so the job has no repository permissions while still allowing it to dispatch the external event.
🤖 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/sync_class_ref.yml:
- Line 19: PAYLOAD_REVISION is hardcoded to "3" causing downstream sync to fetch
the wrong engine revision; replace the literal with the actual merge/commit SHA
by setting PAYLOAD_REVISION to the workflow runtime expression (e.g. use the PR
merge commit then fallback to the current commit): PAYLOAD_REVISION: ${{
github.event.pull_request.merge_commit_sha || github.sha }} so downstream uses
the real engine revision instead of the test value; update the value where
PAYLOAD_REVISION is defined in the workflow.
- Line 18: The workflow sets PAYLOAD_AUTHOR using the nonexistent github.author
context so it will be empty; update the environment assignment for
PAYLOAD_AUTHOR to use a valid context—replace github.author with either
github.actor (the workflow triggerer) or github.event.pull_request.user.login
(the PR author) depending on intent, e.g. change PAYLOAD_AUTHOR: ${{
github.author }} to PAYLOAD_AUTHOR: ${{ github.actor }} or PAYLOAD_AUTHOR: ${{
github.event.pull_request.user.login }} and ensure any downstream consumers of
PAYLOAD_AUTHOR expect the chosen value.
- Line 16: Replace the third-party action reference
passeidireto/trigger-external-workflow-action@main with a pinned immutable
reference (specific commit SHA or release tag) to eliminate the security risk of
tracking `@main`; update the workflow to use the chosen SHA or tag for the uses
entry and ensure any secrets usage (e.g., secrets.REDOT_DOCS_SITE_PAT) remains
unchanged so the workflow continues to run with the same credentials.
---
Nitpick comments:
In @.github/workflows/sync_class_ref.yml:
- Around line 10-14: The job named "trigger" currently has no explicit
permissions and so inherits repository defaults; add a job-level permissions
declaration to enforce least privilege by inserting a permissions: none (or an
empty permissions map) under the "trigger" job so the job has no repository
permissions while still allowing it to dispatch the external event.
.github/workflows/sync_class_ref.yml
Outdated
| - uses: passeidireto/trigger-external-workflow-action@main | ||
| env: | ||
| PAYLOAD_AUTHOR: ${{ github.author }} | ||
| PAYLOAD_REVISION: "3" |
There was a problem hiding this comment.
PAYLOAD_REVISION: "3" is a hardcoded placeholder — downstream sync will target the wrong engine revision
The literal "3" is clearly a test value left over from iteration. The downstream redot-docs-site workflow presumably uses this to know which engine revision to fetch the class DB from. It should reference the actual merge commit SHA.
🐛 Proposed fix
- PAYLOAD_REVISION: "3"
+ PAYLOAD_REVISION: ${{ github.event.pull_request.merge_commit_sha }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PAYLOAD_REVISION: "3" | |
| PAYLOAD_REVISION: ${{ github.event.pull_request.merge_commit_sha }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/sync_class_ref.yml at line 19, PAYLOAD_REVISION is
hardcoded to "3" causing downstream sync to fetch the wrong engine revision;
replace the literal with the actual merge/commit SHA by setting PAYLOAD_REVISION
to the workflow runtime expression (e.g. use the PR merge commit then fallback
to the current commit): PAYLOAD_REVISION: ${{
github.event.pull_request.merge_commit_sha || github.sha }} so downstream uses
the real engine revision instead of the test value; update the value where
PAYLOAD_REVISION is defined in the workflow.
There was a problem hiding this comment.
Hey @17chuchu , what is the purpose of PAYLOAD_REVISION: "3" here?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
…-Engine#4: Remove unneeded changes.
Redot-Engine#4: Test" This reverts commit 031dcc9.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/sync_class_ref.yml:
- Line 4: The workflow includes a manual trigger (workflow_dispatch) but the
job's if condition uses github.event.pull_request.merged == true which is always
false for manual dispatch; either remove the workflow_dispatch trigger if manual
runs aren't needed, or update the job conditional that references
github.event.pull_request.merged == true to first check the event type (e.g.,
github.event_name == 'pull_request' && github.event.pull_request.merged == true)
or allow workflow_dispatch (e.g., github.event_name == 'workflow_dispatch' ||
(github.event_name == 'pull_request' && github.event.pull_request.merged ==
true)) so the job runs when manually triggered; locate the workflow_dispatch
entry and the job if condition in the same file and adjust accordingly.
---
Duplicate comments:
In @.github/workflows/sync_class_ref.yml:
- Line 19: The workflow sets PAYLOAD_AUTHOR using an invalid context
github.author so it will be empty; update the PAYLOAD_AUTHOR assignment to use a
valid context such as github.actor (the user who triggered the workflow) or
github.event.pull_request.user.login (the PR author) wherever PAYLOAD_AUTHOR is
defined in the workflow to ensure the author value is populated.
- Line 17: Replace the mutable reference to the third-party GitHub Action used
in the workflow (the line containing "uses:
passeidireto/trigger-external-workflow-action@main") with an immutable commit
SHA for that action; locate the uses:
passeidireto/trigger-external-workflow-action@main entry and pin it to a
specific commit SHA (e.g.,
passeidireto/trigger-external-workflow-action@<commit-sha>) so the workflow no
longer relies on `@main` and therefore does not accept supply-chain changes for
secrets like REDOT_DOCS_SITE_PAT.
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/sync_class_ref.yml:
- Line 8: The workflow currently triggers on pull_request closed events without
checking whether the PR was merged; update the trigger to keep on: pull_request
with types: [closed] and add a merge-guard using an if conditional (e.g. if:
github.event.pull_request.merged == true) at the job level so downstream
repository_dispatch only runs for merged PRs; apply the same guard to the other
affected job/step entries referenced (lines 12-14) so
rejected/closed-but-unmerged PRs do not trigger the dispatch.
- Line 9: The workflow's branches filter currently uses a scalar string
("branches: master") which should be a sequence; update the branches key in the
workflow (the "branches" filter) to use YAML list syntax (e.g., convert to an
explicit sequence form) so GitHub Actions parses branch filters correctly.
- Line 20: The workflow currently targets the author's fork via the repository
key value "17chuchu/redot-docs-site"; update the repository: entry in
.github/workflows/sync_class_ref.yml to the org repo
"Redot-Engine/redot-docs-site" so the downstream dispatch uses the correct
target and the PAT scope matches the target repository.
---
Duplicate comments:
In @.github/workflows/sync_class_ref.yml:
- Line 18: The workflow sets PAYLOAD_AUTHOR using the invalid context
github.author so PAYLOAD_AUTHOR is always empty; update the environment
assignment for PAYLOAD_AUTHOR to use a valid context such as
github.event.pull_request.user.login (for PRs) or github.actor (for general
triggers) to ensure the author/actor is populated; locate the PAYLOAD_AUTHOR
line in the workflow file and replace github.author with the appropriate valid
context (choose github.event.pull_request.user.login if you want PR author
fallback, or github.actor if you want the workflow triggerer in all cases).
- Line 16: Replace the unpinned GitHub Action reference
"passeidireto/trigger-external-workflow-action@main" with a specific commit SHA
or an immutable release tag to eliminate the security risk; locate the usage of
passeidireto/trigger-external-workflow-action in the workflow, pick a known-good
commit SHA (or published tag) from that repository and update the reference
(e.g., passeidireto/trigger-external-workflow-action@<SHA_OR_TAG>), then verify
the workflow still functions with secrets.REDOT_DOCS_SITE_PAT and commit the
change.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/sync_class_ref.yml (1)
16-16: Consider pinning the action to a commit SHA for immutability
@v1.0.0is a mutable tag. While the action repository is archived and unlikely to change, pinning to a commit SHA guarantees immutability and follows GitHub's security best practices. The action has access tosecrets.REDOT_DOCS_SITE_PAT, making it a sensitive operation that benefits from explicit version control.Resolve the commit SHA for
v1.0.0by checking the action's releases page on GitHub and update the reference accordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/sync_class_ref.yml at line 16, Replace the mutable tag reference "uses: passeidireto/trigger-external-workflow-action@v1.0.0" with the corresponding commit SHA (resolved from the action’s GitHub releases/tags page) so the workflow pins the action immutably; update the line that currently contains passeidireto/trigger-external-workflow-action@v1.0.0 to passeidireto/trigger-external-workflow-action@<commit-sha>.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/sync_class_ref.yml:
- Line 9: The workflow's branches filter uses a scalar ("branches: master") but
should be a YAML sequence; update the branches key to use list syntax (e.g.,
change the branches declaration in the workflow to a sequence such as branches:
[master] or the equivalent block-style list) so the GitHub Actions runner
correctly interprets the branch filter.
- Line 20: The workflow sets the repository target to the contributor's fork
(repository: 17chuchu/redot-docs-site); change that value to the organization
repo used by the PAT (repository: Redot-Engine/redot-docs-site) so the dispatch
runs against the correct repo; update the repository field in
.github/workflows/sync_class_ref.yml to reference Redot-Engine/redot-docs-site
(search for the repository: entry) and verify the dispatch action uses that
updated repository value.
- Around line 12-14: The workflow currently fires the docs repository_dispatch
for any pull_request closed event; update the job that sends the
repository_dispatch (the job/step that calls repository_dispatch) to guard
against non-merged PRs by adding an if condition that only allows
non-pull_request events or merged pull requests, e.g. use an if expression
referencing github.event_name and github.event.pull_request.merged (for example:
if: github.event_name != 'pull_request' || github.event.pull_request.merged ==
true) so the downstream repository_dispatch only runs for merged PRs or manual
workflow_dispatch.
---
Nitpick comments:
In @.github/workflows/sync_class_ref.yml:
- Line 16: Replace the mutable tag reference "uses:
passeidireto/trigger-external-workflow-action@v1.0.0" with the corresponding
commit SHA (resolved from the action’s GitHub releases/tags page) so the
workflow pins the action immutably; update the line that currently contains
passeidireto/trigger-external-workflow-action@v1.0.0 to
passeidireto/trigger-external-workflow-action@<commit-sha>.
Arctis-Fireblight
left a comment
There was a problem hiding this comment.
Approved.
Gonna hold off on merging for a bit till we get the Cloudflare deployment working, to hopefully get to see this in action.
New Features
Add CI to detect changes from merged pull-request then trigger CI on redot-docs-site to generate class documents. Implemented as part of this issue
Post merge steps
Generate a Fine-grained personal access tokens
The CI will need a token to access CI on redot-docs-site repo. The token will need to be created by the owner or admins of redot-docs-site.
Generate a Fine-grained personal access tokens
The CI should now be able to trigger sync_class_ref CI on redot-docs-site.
Summary by CodeRabbit