ci: fix documentation artefacts automatically after merge#8024
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughAdds a GitHub Actions workflow that regenerates public and private API documentation artefacts on changes to Estimated code review effort: 3 (Moderate) | ~20 minutes 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 |
github-actions
Bot
added
api
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8024 +/- ##
========================================
Coverage 98.64% 98.64%
========================================
Files 1501 1506 +5
Lines 59397 59572 +175
========================================
+ Hits 58591 58766 +175
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
github-actions
Bot
added
ci-cd
1235b2e to
380c28c
Compare
github-actions
Bot
added
ci-cd
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HWahXvKyob4DTBgmGzV1gm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d104773 to
f38778a
Compare
github-actions
Bot
added
ci-cd
This reverts commit 180332d.
2804a78 to
e61a1a6
Compare
github-actions
Bot
added
ci-cd
github-actions
Bot
added
ci-cd
Docker builds report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e148b71d-cbcf-442b-b625-2909e46046a1
📒 Files selected for processing (7)
.github/workflows/api-fix-docs-artefacts.yml.github/workflows/api-pull-request.yml.github/workflows/api-tests-with-private-packages.ymlapi/experimentation/tasks.pyapi/experimentation/views.pyapi/features/views.pyapi/segment_membership/metrics.py
💤 Files with no reviewable changes (2)
- .github/workflows/api-pull-request.yml
- .github/workflows/api-tests-with-private-packages.yml
| permissions: | ||
| contents: read # For actions/checkout | ||
| id-token: write # For CodeArtifact OIDC |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔵 Trivial | 💤 Low value
Restrict id-token: write permission to the job level.
Applying id-token: write at the workflow level grants broad access to all jobs in the workflow, which violates the principle of least privilege. While there is only one job here, it is best practice to move the permissions block to the specific job that requires it.
🛡️ Proposed refactor
Remove the global permissions block:
-permissions:
- contents: read # For actions/checkout
- id-token: write # For CodeArtifact OIDCAnd add it to the fix-docs-artefacts job:
jobs:
fix-docs-artefacts:
runs-on: depot-ubuntu-latest
name: Fix Documentation Artefacts
+ permissions:
+ contents: read
+ id-token: write🧰 Tools
🪛 zizmor (1.26.1)
[error] 5-5: overly broad permissions (excessive-permissions): id-token: write is overly broad at the workflow level
(excessive-permissions)
Source: Linters/SAST tools
| - name: Commit and push fixes | ||
| run: | | ||
| git add --all | ||
| if git diff --cached --quiet; then | ||
| echo "Documentation artefacts are up to date." | ||
| exit 0 | ||
| fi | ||
| git config user.name '${{ steps.app-token.outputs.app-slug }}[bot]' | ||
| git config user.email '${{ steps.app-token.outputs.app-slug }}[bot]@users.noreply.github.com' | ||
| git commit --message 'chore: Update documentation artefacts' | ||
| git push || { git pull --rebase && git push; } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Improve commit step reliability and prevent redundant CI runs.
- Prevent Redundant Triggers: Because this workflow uses a GitHub App token to push changes, the push will re-trigger this and any other
on: pushworkflows. Appending[skip ci]to the commit message prevents these redundant executions, saving CI resources in direct alignment with the PR objectives. - Working Directory: The default working directory for the job is
api/. Whilegit add --alloperates on the entire working tree regardless of the current directory (in Git 2.0+), it is clearer and safer to explicitly set the working directory to the repository root for repository-wide Git commands to avoid any pathspec ambiguity with artefacts generated in../docs.
♻️ Proposed refactor
- name: Commit and push fixes
+ working-directory: ${{ github.workspace }}
run: |
git add --all
if git diff --cached --quiet; then
echo "Documentation artefacts are up to date."
exit 0
fi
git config user.name '${{ steps.app-token.outputs.app-slug }}[bot]'
git config user.email '${{ steps.app-token.outputs.app-slug }}[bot]`@users.noreply.github.com`'
- git commit --message 'chore: Update documentation artefacts'
+ git commit --message 'chore: Update documentation artefacts [skip ci]'
git push || { git pull --rebase && git push; }📝 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: Commit and push fixes | |
| run: | | |
| git add --all | |
| if git diff --cached --quiet; then | |
| echo "Documentation artefacts are up to date." | |
| exit 0 | |
| fi | |
| git config user.name '${{ steps.app-token.outputs.app-slug }}[bot]' | |
| git config user.email '${{ steps.app-token.outputs.app-slug }}[bot]@users.noreply.github.com' | |
| git commit --message 'chore: Update documentation artefacts' | |
| git push || { git pull --rebase && git push; } | |
| - name: Commit and push fixes | |
| working-directory: ${{ github.workspace }} | |
| run: | | |
| git add --all | |
| if git diff --cached --quiet; then | |
| echo "Documentation artefacts are up to date." | |
| exit 0 | |
| fi | |
| git config user.name '${{ steps.app-token.outputs.app-slug }}[bot]' | |
| git config user.email '${{ steps.app-token.outputs.app-slug }}[bot]`@users.noreply.github.com`' | |
| git commit --message 'chore: Update documentation artefacts [skip ci]' | |
| git push || { git pull --rebase && git push; } |
🧰 Tools
🪛 zizmor (1.26.1)
[info] 75-75: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 76-76: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Failed testsfirefox › tests/onboarding-tests.pw.ts › Onboarding › New user connects via the single-page onboarding flow @oss
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Responding to the original reasoning:
Why we didn't use pre-commit's CI?
Because it runs in an isolated environment lacking the runtime to auto-generate docs, and it doesn't have access to private packages.
Is there a way to run pre-commit hooks in a runtime we need in CI? This would allow us to rely on hooks more rather than less, and lean on standardised CI vs introducing another custom workflow.
Why not push changes to the PR?
Because 1. it uses less resources per work; and 2. it allows community PR not to fail CI.
This approach decouples autogenerated changes from the actual changes that cause them; i.e. I do want to see specific changes to the API schema, should the PR introduce them, and review them together with the code.
The concern number 2 should be addressed if we solve the running hooks part — our GitHub app's credentials will be used to run them on any PR.
If we decide to keep with the current workflow, we'll have to avoid pushing directly to main.
| git commit --message 'chore: Update documentation artefacts' | ||
| git push || { git pull --rebase && git push; } |
There was a problem hiding this comment.
You mean having app pushing to main?
There was a problem hiding this comment.
I mean having anything pushing to main bypassing the PR approval mechanism.
|
Caution Review failedAn error occurred during the review process. Please try again later. 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 |
2 participants
Human text.
Closes #8021
The team has been made responsible to include auto-generated documentation changes in their PRs, leading to some time wasted in back-and-forth make targets and git-fu — not to mention the need to authenticate to AWS for CodeArtefact (private packages). This breaks us free!
Changes
Review effort: 2/5
FAQ
Because it runs in an isolated environment lacking the runtime to auto-generate docs, and it doesn't have access to private packages.
Because 1. it uses less resources per work; and 2. it allows community PR not to fail CI.