-
Notifications
You must be signed in to change notification settings - Fork 16
Improve CI workflows for docs and publishing #187
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| needs: build | ||
| if: github.event_name == 'push' && github.ref == 'refs/heads/main' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/download-artifact@v4 | ||
| with: { name: site, path: site } | ||
| - name: Deploy to GitHub Pages (main) | ||
| uses: peaceiris/actions-gh-pages@v4 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| publish_branch: gh-pages | ||
| publish_dir: ./site | ||
| force_orphan: true | ||
|
|
||
| - name: Deploy to GitHub Pages | ||
| deploy-preview: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
The fix is to add the permissions: block to the workflow. Best practice is to add it at the root level unless jobs require different permissions. For this workflow, the deploy-main and deploy-preview jobs deploy to GitHub Pages using GITHUB_TOKEN, which only needs restricted permissions such as pages: write and contents: read. The build job does not need any write permissions, so read-only access is sufficient.
- At the root of the YAML file (top level), insert:
permissions: contents: read pages: write
This approach sets minimal permissions for all jobs, giving only read access to repository contents and write access to GitHub Pages.
Alternatively, to be even more strict, set permissions per job: build gets contents: read; deploy-main and deploy-preview get contents: read and pages: write. However, setting it at the workflow root level is sufficiently secure for this case.
No other code or logic changes are needed. No new imports or external dependencies (other than what is already used in the workflow) are required.
-
Copy modified lines R2-R4
| @@ -1,4 +1,7 @@ | ||
| name: Docs — Build & Preview | ||
| permissions: | ||
| contents: read | ||
| pages: write | ||
|
|
||
| on: | ||
| push: |
| needs: build | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/download-artifact@v4 | ||
| with: { name: site, path: site } | ||
| - name: Deploy preview under subfolder | ||
| uses: peaceiris/actions-gh-pages@v4 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| publish_branch: gh-pages | ||
| publish_dir: ./site | ||
| force_orphan: true | ||
| destination_dir: preview/${{ github.head_ref || github.ref_name }}/${{ needs.build.outputs.short_sha }} | ||
| keep_files: true # keep previous previews | ||
| # DO NOT set force_orphan here | ||
| - name: Print preview URL | ||
| run: | |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #187 +/- ##
=======================================
Coverage 89.23% 89.23%
=======================================
Files 14 14
Lines 2007 2007
=======================================
Hits 1791 1791
Misses 216 216 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - name: Deploy preview under subfolder | ||
| uses: peaceiris/actions-gh-pages@v4 | ||
| with: | ||
| github_token: ${{ secrets.GITHUB_TOKEN }} | ||
| publish_branch: gh-pages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Skip gh-pages deploy when pull_request token lacks write access
The preview deploy job runs for every pull_request but immediately pushes to gh-pages with secrets.GITHUB_TOKEN. For PRs from forks the GitHub-provided token is read-only, so peaceiris/actions-gh-pages cannot write and the workflow will fail, blocking doc-only PRs from external contributors. Consider skipping this deploy when github.event.pull_request.head.repo.fork is true or using a separate token.
Useful? React with 👍 / 👎.
| git config user.email github-actions@github.com | ||
| git add CHANGELOG.md | ||
| # Avoid CI cycles | ||
| git commit -m "Changelog: add notes for ${TAG} [skip ci]" || echo "No changelog changes to commit" | ||
| git push origin main || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensure CHANGELOG update actually lands on main
During the release workflow this step commits CHANGELOG updates while still on the tag’s detached HEAD and then runs git push origin main || true. Because no local main branch exists at this point the push is a no-op, and subsequent git checkout main resets the worktree, so the newly generated release notes never reach the main branch (they only live on the temporary release branch). Releases therefore leave CHANGELOG.md stale on main.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughRefactors GitHub Actions workflows to modernize tooling and deployment orchestration. The documentation workflow restructures into separate build and deploy jobs with artifact-based deployment paths. The publish workflow replaces direct tool invocations with UV-based execution and introduces automated changelog generation from release notes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/publish.yml (1)
65-88: CRITICAL: CHANGELOG updates won't reach main due to detached HEAD state.The workflow commits CHANGELOG changes while on a detached HEAD (at the release tag), then attempts
git push origin main || true. Since no localmainbranch exists at this point, the push is silently skipped. The subsequentgit checkout main(line 106) resets the worktree, leaving CHANGELOG stale on main.This is the issue flagged in the previous review. To fix, checkout
mainbefore committing the CHANGELOG update:- - name: Commit and push CHANGELOG update - env: - TAG: ${{ steps.notes.outputs.tag }} - run: | - git config user.name github-actions - git config user.email github-actions@github.com + - name: Commit and push CHANGELOG update + env: + TAG: ${{ steps.notes.outputs.tag }} + run: | + git config user.name github-actions + git config user.email github-actions@github.com + git checkout main git add CHANGELOG.md # Avoid CI cycles git commit -m "Changelog: add notes for ${TAG} [skip ci]" || echo "No changelog changes to commit" git push origin main || true.github/workflows/docs-gh-pages.yml (2)
82-96: CRITICAL: deploy-preview will fail for fork PRs due to read-only token.The previous review flagged this: the
deploy-previewjob runs for all PRs, but when triggered from a fork,secrets.GITHUB_TOKENis read-only, causingpeaceiris/actions-gh-pagesto fail. This blocks documentation-only PRs from external contributors.Add a guard condition to skip deployment for fork PRs:
deploy-preview: needs: build - if: github.event_name == 'pull_request' + if: github.event_name == 'pull_request' && !github.event.pull_request.head.repo.fork runs-on: ubuntu-latest + permissions: + contents: write steps:Consider adding a separate step to post the preview URL as a comment for fork PRs (optional enhancement).
67-80: Add permissions block to deploy-main job.The job lacks an explicit permissions statement for GITHUB_TOKEN. Since this job publishes to gh-pages, add a minimal permissions block to limit token scope. This aligns with the permissions patterns used elsewhere in the project (e.g.,
publish.yml).Add permissions to this job:
deploy-main: needs: build if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest + permissions: + contents: write steps:Note: The
deploy-previewjob has the same issue and should also receive a permissions block.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/docs-gh-pages.yml(1 hunks).github/workflows/publish.yml(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: projectmesa/mesa-frames PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T18:41:11.772Z
Learning: In pull requests, link issues, summarize changes, note API impacts, and add/adjust tests and documentation
📚 Learning: 2025-12-08T18:41:11.772Z
Learnt from: CR
Repo: projectmesa/mesa-frames PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T18:41:11.772Z
Learning: Use `docs/` directory for MkDocs and Sphinx content for user and API documentation
Applied to files:
.github/workflows/docs-gh-pages.yml
📚 Learning: 2025-12-08T18:41:11.772Z
Learnt from: CR
Repo: projectmesa/mesa-frames PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-08T18:41:11.772Z
Learning: Preview documentation using `uv run mkdocs serve`
Applied to files:
.github/workflows/docs-gh-pages.yml
🪛 actionlint (1.7.9)
.github/workflows/publish.yml
43-43: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/docs-gh-pages.yml
99-99: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🪛 GitHub Check: CodeQL
.github/workflows/docs-gh-pages.yml
[warning] 68-82: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
[warning] 83-100: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
🔇 Additional comments (8)
.github/workflows/publish.yml (6)
20-28: UV tooling migration looks good.The setup and version extraction are implemented correctly. The use of
uvx hatchis the correct invocation pattern via uv.
29-41: Build, test, and verification steps are correct.The UV-based commands are properly formatted. The PyPI verification flow is sound.
48-64: Changelog generation from release notes is well-structured.The script safely extracts release data from the GitHub API context with fallback to GITHUB_REF, validates the payload, and handles missing release body gracefully with a clear error message.
89-106: Version branch creation logic is sound, but depends on CHANGELOG fix.Once the CHANGELOG commit issue (lines 79-88) is resolved by checking out
mainfirst, this section will execute correctly. The version branch recreation and main checkout are appropriate.
107-121: Version bumping and commit are correct.The UV-based version commands are properly sequenced, and including CHANGELOG.md in the commit is appropriate. This assumes the main branch checkout from the CHANGELOG step completes successfully.
43-43: Update softprops/action-gh-release to latest version.Static analysis flagged
action-gh-release@v1as too old. Please verify and update to the latest stable version..github/workflows/docs-gh-pages.yml (2)
1-13: Workflow triggers are well-configured.Path-based filters appropriately constrain runs to doc changes only. The split between push (main) and pull_request (all branches) is correct.
16-66: Build job is well-structured and defensive.The job properly exports
short_shafor downstream use, defensively handles notebook conversion with globstar pattern matching, and artifacts the built site. The separation of build from deploy jobs is a good pattern improvement.
| - name: Print preview URL | ||
| run: | | ||
| echo "Preview: https://${{ github.repository_owner }}.github.io/$(basename ${{ github.repository }})/preview/${{ github.head_ref || github.ref_name }}/${{ needs.build.outputs.short_sha }}/" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix script injection risk: pass github.head_ref via environment variable.
The inline script uses github.head_ref directly, creating a script injection vulnerability. Untrusted values from PRs could contain shell metacharacters. Pass it through an environment variable instead:
- name: Print preview URL
+ env:
+ HEAD_REF: ${{ github.head_ref || github.ref_name }}
run: |
- echo "Preview: https://${{ github.repository_owner }}.github.io/$(basename ${{ github.repository }})/preview/${{ github.head_ref || github.ref_name }}/${{ needs.build.outputs.short_sha }}/"
+ echo "Preview: https://${{ github.repository_owner }}.github.io/$(basename ${{ github.repository }})/preview/${HEAD_REF}/${{ needs.build.outputs.short_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.
| - name: Print preview URL | |
| run: | | |
| echo "Preview: https://${{ github.repository_owner }}.github.io/$(basename ${{ github.repository }})/preview/${{ github.head_ref || github.ref_name }}/${{ needs.build.outputs.short_sha }}/" | |
| - name: Print preview URL | |
| env: | |
| HEAD_REF: ${{ github.head_ref || github.ref_name }} | |
| run: | | |
| echo "Preview: https://${{ github.repository_owner }}.github.io/$(basename ${{ github.repository }})/preview/${HEAD_REF}/${{ needs.build.outputs.short_sha }}/" |
🧰 Tools
🪛 actionlint (1.7.9)
99-99: "github.head_ref" is potentially untrusted. avoid using it directly in inline scripts. instead, pass it through an environment variable. see https://docs.github.com/en/actions/reference/security/secure-use#good-practices-for-mitigating-script-injection-attacks for more details
(expression)
🪛 GitHub Check: CodeQL
[warning] 83-100: Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
🤖 Prompt for AI Agents
.github/workflows/docs-gh-pages.yml lines 98-100: the current run step injects
github.head_ref directly into a shell string, risking script injection from
untrusted PR refs; instead define an environment variable (e.g., PREVIEW_REF)
set to the expression ${{ github.head_ref || github.ref_name }} in the step's
env block and reference that env var inside the run script (use double quotes
and the shell env variable) so the workflow does not perform direct
interpolation of the GitHub context into the shell command.
EwoutH
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven’t reviewed line by line, but conceptually this seems useful!
Documentation:
preview/{branch}/{short_sha}/iffdocs/*files are modifieddocs/general/**/*.pyand converts each to.ipynbviajupytext --to notebook. This allows to have better git history for tutorialsPublishing:
uvto managehatchCHANGELOG.mdautomatic managementSummary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.