ci(docs): pass github token to docs workflows#452
Conversation
📝 WalkthroughWalkthroughThis pull request introduces end-to-end CI/CD infrastructure for documentation deployment. It adds two GitHub Actions workflows: one for deploying documentation to a remote server with optional symlink switching and Nginx reloading, and another for verifying deployment artifacts. A guide document outlines the complete setup, including server configuration, TLS certificates, GitHub secrets, and workflow operation procedures. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 5
🧹 Nitpick comments (4)
scripts/docs/README.md (1)
45-54: Consider documenting theGITHUB_TOKENusage.Since the PR objective is to pass
GITHUB_TOKENto avoid rate limits during docs builds, it would be helpful to mention that the workflows use the built-inGITHUB_TOKENsecret (automatically provided by GitHub Actions) for authenticated API access during changelog/metadata generation. This clarifies to readers that no additional secret configuration is needed for this purpose.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/docs/README.md` around lines 45 - 54, Add a short note to the "GitHub Secrets Required" section explaining that workflows also use the built-in GITHUB_TOKEN (automatically provided by GitHub Actions) for authenticated API access during changelog/metadata generation to avoid rate limits, so no additional secret needs to be set for that purpose; reference the existing secret names such as DOCS_DEPLOY_HOST, DOCS_DEPLOY_SSH_KEY, and DOCS_DEPLOY_BASE_DIR to show this is an additional built-in token used by the docs build workflows..github/workflows/deploy-docs.yaml (2)
21-24: Unused input:source_shais never referenced.The
source_shainput is declared but not used anywhere in the workflow. Either remove it or use it for SHA verification (e.g., to confirm the checkout matches the expected commit).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-docs.yaml around lines 21 - 24, The workflow declares an unused input named source_sha; either remove that input block or wire it into the checkout/verification flow: if you keep it, pass inputs.source_sha into the actions/checkout ref (or add a step named "Verify source SHA" that runs git rev-parse HEAD and compares it to inputs.source_sha) and fail the job when they differ; if you remove it, simply delete the source_sha input declaration to avoid an unused parameter.
70-73: Same suggestion: usepnpm/action-setupfor consistency.Same as the verify workflow—consider using
pnpm/action-setupfor more reliable pnpm installation and caching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy-docs.yaml around lines 70 - 73, Replace the manual global install of pnpm in the "Install dependencies" step with the pnpm GitHub Action to match the verify workflow; specifically, remove the "npm i -g pnpm" run and instead call the action (pnpm/action-setup@v2) in that step, then run "pnpm install" (optionally enabling the action's cache inputs) so pnpm is installed reliably and caching behavior mirrors the verify workflow..github/workflows/docs-deploy-verify.yaml (1)
45-48: Consider usingpnpm/action-setupfor consistent pnpm installation.Installing pnpm globally via npm works but is less idiomatic for GitHub Actions. Using
pnpm/action-setupensures version consistency and better caching integration.Suggested improvement
+ - name: Setup pnpm + uses: pnpm/action-setup@v4 + with: + version: 9 + - name: Setup node uses: actions/setup-node@v4 with: node-version: 20 + cache: 'pnpm' - name: Install dependencies - run: | - npm i -g pnpm - pnpm install + run: pnpm install🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docs-deploy-verify.yaml around lines 45 - 48, Replace the current "Install dependencies" step that runs "npm i -g pnpm" and "pnpm install" with the pnpm GitHub Action: use the step named "Install dependencies" to call "pnpm/action-setup" (e.g., uses: pnpm/action-setup@v2 and set the version with with: version) and then run "pnpm install" in the following run step; this ensures a consistent pnpm version and better caching integration while keeping the step name "Install dependencies" for clarity.
🤖 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/deploy-docs.yaml:
- Around line 171-173: The deploy step runs "nginx -t" and "systemctl reload
nginx" as the deploy user which normally requires root; update the workflow to
invoke these commands with sudo (e.g., use "sudo nginx -t" and "sudo systemctl
reload nginx") or ensure the DOCS_DEPLOY_USER has passwordless sudo for those
binaries, and add a note to scripts/docs/README.md documenting the required
sudoers entry and environment variables (DOCS_DEPLOY_USER, DOCS_DEPLOY_HOST,
DOCS_DEPLOY_PORT) so the deployment will succeed without interactive password
prompts.
- Around line 41-43: The workflow currently sets concurrency with
cancel-in-progress: true which can abort an ongoing release and leave
deployments partial; update the concurrency block (the group: docs-release-${{
inputs.version }} / cancel-in-progress setting) to use cancel-in-progress: false
(or remove the true value) so that an initiated release is allowed to finish
before a new one starts, ensuring atomic completion of the release job.
In @.github/workflows/docs-deploy-verify.yaml:
- Around line 14-18: The workflow input `source_ref` has a typo in its default
value ('refs/heads/updata-2602'); update the default for the `source_ref` input
to a correct branch name (e.g., 'refs/heads/update-2602' or a stable branch like
'refs/heads/main') or remove the default since `source_ref` is required; locate
the `source_ref` block in the docs-deploy-verify workflow and change the
`default` field accordingly to the intended ref.
- Around line 122-125: The workflow's remote verification uses a check for
'$REMOTE_DIR/.vitepress' which will never exist because rsync uploads the
contents of apps/docs/.vitepress/dist/ into $REMOTE_DIR; update the ssh test
block in .github/workflows/docs-deploy-verify.yaml to remove the non-existent
"test -d '$REMOTE_DIR/.vitepress'" check and instead verify presence of
generated artifacts (e.g., keep "test -d '$REMOTE_DIR/assets'" and "test -f
'$REMOTE_DIR/index.html'" or add checks for other dist files) so the SSH command
tests the actual uploaded files under $REMOTE_DIR.
In `@scripts/docs/README.md`:
- Around line 24-26: Remove the mkdir that pre-creates the "current" directories
so workflows can atomically replace "current" with a symlink; specifically,
update scripts/docs/README.md to stop calling "sudo mkdir -p
/var/www/element-plus-x/v1/current /var/www/element-plus-x/v2/current" and only
create the "releases" paths, and add a short note that deploy-docs.yaml uses "ln
-sfn" to set "current" as a symlink so it must not exist as a directory.
---
Nitpick comments:
In @.github/workflows/deploy-docs.yaml:
- Around line 21-24: The workflow declares an unused input named source_sha;
either remove that input block or wire it into the checkout/verification flow:
if you keep it, pass inputs.source_sha into the actions/checkout ref (or add a
step named "Verify source SHA" that runs git rev-parse HEAD and compares it to
inputs.source_sha) and fail the job when they differ; if you remove it, simply
delete the source_sha input declaration to avoid an unused parameter.
- Around line 70-73: Replace the manual global install of pnpm in the "Install
dependencies" step with the pnpm GitHub Action to match the verify workflow;
specifically, remove the "npm i -g pnpm" run and instead call the action
(pnpm/action-setup@v2) in that step, then run "pnpm install" (optionally
enabling the action's cache inputs) so pnpm is installed reliably and caching
behavior mirrors the verify workflow.
In @.github/workflows/docs-deploy-verify.yaml:
- Around line 45-48: Replace the current "Install dependencies" step that runs
"npm i -g pnpm" and "pnpm install" with the pnpm GitHub Action: use the step
named "Install dependencies" to call "pnpm/action-setup" (e.g., uses:
pnpm/action-setup@v2 and set the version with with: version) and then run "pnpm
install" in the following run step; this ensures a consistent pnpm version and
better caching integration while keeping the step name "Install dependencies"
for clarity.
In `@scripts/docs/README.md`:
- Around line 45-54: Add a short note to the "GitHub Secrets Required" section
explaining that workflows also use the built-in GITHUB_TOKEN (automatically
provided by GitHub Actions) for authenticated API access during
changelog/metadata generation to avoid rate limits, so no additional secret
needs to be set for that purpose; reference the existing secret names such as
DOCS_DEPLOY_HOST, DOCS_DEPLOY_SSH_KEY, and DOCS_DEPLOY_BASE_DIR to show this is
an additional built-in token used by the docs build workflows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 37835f05-7a90-466f-aac5-c2a178c23a8a
📒 Files selected for processing (3)
.github/workflows/deploy-docs.yaml.github/workflows/docs-deploy-verify.yamlscripts/docs/README.md
| concurrency: | ||
| group: docs-release-${{ inputs.version }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
Reconsider cancel-in-progress: true for release workflows.
Canceling an in-progress release could leave the deployment in a partial state (e.g., files uploaded but symlink not switched). For release workflows, cancel-in-progress: false is safer to ensure each release completes atomically.
Suggested fix
concurrency:
group: docs-release-${{ inputs.version }}
- cancel-in-progress: true
+ cancel-in-progress: false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-docs.yaml around lines 41 - 43, The workflow
currently sets concurrency with cancel-in-progress: true which can abort an
ongoing release and leave deployments partial; update the concurrency block (the
group: docs-release-${{ inputs.version }} / cancel-in-progress setting) to use
cancel-in-progress: false (or remove the true value) so that an initiated
release is allowed to finish before a new one starts, ensuring atomic completion
of the release job.
| run: | | ||
| PORT="${DOCS_DEPLOY_PORT:-22}" | ||
| ssh -p "$PORT" "$DOCS_DEPLOY_USER@$DOCS_DEPLOY_HOST" "nginx -t && systemctl reload nginx" |
There was a problem hiding this comment.
nginx -t and systemctl reload nginx likely require sudo.
These commands typically require root privileges. Unless the deploy user has passwordless sudo configured (which should be documented), this step will fail.
Suggested fix
PORT="${DOCS_DEPLOY_PORT:-22}"
- ssh -p "$PORT" "$DOCS_DEPLOY_USER@$DOCS_DEPLOY_HOST" "nginx -t && systemctl reload nginx"
+ ssh -p "$PORT" "$DOCS_DEPLOY_USER@$DOCS_DEPLOY_HOST" "sudo nginx -t && sudo systemctl reload nginx"Also consider documenting in scripts/docs/README.md that the deploy user needs passwordless sudo for nginx commands:
echo "deploy_user ALL=(ALL) NOPASSWD: /usr/sbin/nginx, /bin/systemctl reload nginx" | sudo tee /etc/sudoers.d/docs-deploy🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/deploy-docs.yaml around lines 171 - 173, The deploy step
runs "nginx -t" and "systemctl reload nginx" as the deploy user which normally
requires root; update the workflow to invoke these commands with sudo (e.g., use
"sudo nginx -t" and "sudo systemctl reload nginx") or ensure the
DOCS_DEPLOY_USER has passwordless sudo for those binaries, and add a note to
scripts/docs/README.md documenting the required sudoers entry and environment
variables (DOCS_DEPLOY_USER, DOCS_DEPLOY_HOST, DOCS_DEPLOY_PORT) so the
deployment will succeed without interactive password prompts.
| source_ref: | ||
| description: 'Git ref to build (branch or tag)' | ||
| required: true | ||
| default: 'refs/heads/updata-2602' | ||
| type: string |
There was a problem hiding this comment.
Typo in default source_ref value.
The default value 'refs/heads/updata-2602' appears to contain a typo—likely intended to be 'refs/heads/update-2602' or another valid branch name. Consider using a more stable default like 'refs/heads/main' or removing the default entirely since source_ref is required.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docs-deploy-verify.yaml around lines 14 - 18, The workflow
input `source_ref` has a typo in its default value ('refs/heads/updata-2602');
update the default for the `source_ref` input to a correct branch name (e.g.,
'refs/heads/update-2602' or a stable branch like 'refs/heads/main') or remove
the default since `source_ref` is required; locate the `source_ref` block in the
docs-deploy-verify workflow and change the `default` field accordingly to the
intended ref.
| ssh -p "$PORT" "$DOCS_DEPLOY_USER@$DOCS_DEPLOY_HOST" "\ | ||
| test -d '$REMOTE_DIR' && \ | ||
| test -f '$REMOTE_DIR/index.html' && \ | ||
| (test -d '$REMOTE_DIR/assets' || test -d '$REMOTE_DIR/.vitepress')" |
There was a problem hiding this comment.
Remote artifact check for .vitepress directory will always fail.
The rsync uploads from apps/docs/.vitepress/dist/ to $REMOTE_DIR/, so the remote directory contains the contents of dist/ (e.g., index.html, assets/), not a .vitepress folder. The check test -d '$REMOTE_DIR/.vitepress' will never succeed.
Suggested fix
ssh -p "$PORT" "$DOCS_DEPLOY_USER@$DOCS_DEPLOY_HOST" "\
test -d '$REMOTE_DIR' && \
test -f '$REMOTE_DIR/index.html' && \
- (test -d '$REMOTE_DIR/assets' || test -d '$REMOTE_DIR/.vitepress')"
+ test -d '$REMOTE_DIR/assets'"📝 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.
| ssh -p "$PORT" "$DOCS_DEPLOY_USER@$DOCS_DEPLOY_HOST" "\ | |
| test -d '$REMOTE_DIR' && \ | |
| test -f '$REMOTE_DIR/index.html' && \ | |
| (test -d '$REMOTE_DIR/assets' || test -d '$REMOTE_DIR/.vitepress')" | |
| ssh -p "$PORT" "$DOCS_DEPLOY_USER@$DOCS_DEPLOY_HOST" "\ | |
| test -d '$REMOTE_DIR' && \ | |
| test -f '$REMOTE_DIR/index.html' && \ | |
| test -d '$REMOTE_DIR/assets'" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docs-deploy-verify.yaml around lines 122 - 125, The
workflow's remote verification uses a check for '$REMOTE_DIR/.vitepress' which
will never exist because rsync uploads the contents of
apps/docs/.vitepress/dist/ into $REMOTE_DIR; update the ssh test block in
.github/workflows/docs-deploy-verify.yaml to remove the non-existent "test -d
'$REMOTE_DIR/.vitepress'" check and instead verify presence of generated
artifacts (e.g., keep "test -d '$REMOTE_DIR/assets'" and "test -f
'$REMOTE_DIR/index.html'" or add checks for other dist files) so the SSH command
tests the actual uploaded files under $REMOTE_DIR.
| sudo mkdir -p /var/www/element-plus-x/v1/releases /var/www/element-plus-x/v2/releases | ||
| sudo mkdir -p /var/www/element-plus-x/v1/current /var/www/element-plus-x/v2/current | ||
| ``` |
There was a problem hiding this comment.
Avoid pre-creating current as a directory—workflows expect it to be a symlink.
Line 25 creates /var/www/element-plus-x/v1/current and v2/current as directories. However, the deploy-docs.yaml workflow (line 144) uses ln -sfn to atomically replace current with a symlink pointing to a release directory. If current exists as a non-empty directory, ln -sfn will create a symlink inside that directory rather than replacing it.
Suggested fix
sudo mkdir -p /var/www/element-plus-x/v1/releases /var/www/element-plus-x/v2/releases
-sudo mkdir -p /var/www/element-plus-x/v1/current /var/www/element-plus-x/v2/current
+# Note: 'current' will be created as a symlink by the first release workflow run📝 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.
| sudo mkdir -p /var/www/element-plus-x/v1/releases /var/www/element-plus-x/v2/releases | |
| sudo mkdir -p /var/www/element-plus-x/v1/current /var/www/element-plus-x/v2/current | |
| ``` | |
| sudo mkdir -p /var/www/element-plus-x/v1/releases /var/www/element-plus-x/v2/releases | |
| # Note: 'current' will be created as a symlink by the first release workflow run |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/docs/README.md` around lines 24 - 26, Remove the mkdir that
pre-creates the "current" directories so workflows can atomically replace
"current" with a symlink; specifically, update scripts/docs/README.md to stop
calling "sudo mkdir -p /var/www/element-plus-x/v1/current
/var/www/element-plus-x/v2/current" and only create the "releases" paths, and
add a short note that deploy-docs.yaml uses "ln -sfn" to set "current" as a
symlink so it must not exist as a directory.
Summary
GITHUB_TOKEN/GH_TOKENinto docs build stepsSummary by CodeRabbit