Ci/refactored ci pipelines#558
Conversation
WalkthroughGitHub Actions CI/CD was restructured: added comprehensive CI docs, new composite actions (changed-packages, deploy-vercel, deploy-docs, build, lint, test, setup-env), new workflows (pull-request, deploy-docs), refactored release workflow, removed legacy workflows/composites, and moved Playwright install out of package postinstall into test steps. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub
participant WF as PR Workflow<br/>(pull-request.yaml)
participant CA as Composite<br/>Actions
participant Turbo as Turbo
participant Cache as Cache
participant Vercel as Vercel
GH->>WF: Pull request opened/updated
WF->>WF: skip-duplicate-check
WF->>CA: changed-packages action
CA->>Turbo: npx turbo build --dry-run=json (with filter)
Turbo-->>CA: package_changed JSON
CA-->>WF: package_changed output
WF->>CA: build action
CA->>Cache: restore build outputs
CA->>CA: npm run build
CA->>Cache: save build outputs
WF->>CA: lint action
CA->>Cache: restore build outputs
CA->>CA: npm run lint
WF->>CA: test action
CA->>Cache: restore build outputs
CA->>CA: npx playwright install --with-deps
CA->>CA: npm run test
alt docs package changed
WF->>CA: deploy-docs action (preview)
CA->>CA: setup-env
CA->>CA: deploy-vercel (preview)
CA->>Vercel: vercel deploy --prebuilt (with token)
Vercel-->>CA: deployment result
end
sequenceDiagram
participant GH as GitHub
participant WF as Docs Workflow<br/>(deploy-docs.yaml)
participant CA as Composite<br/>Actions
participant Vercel as Vercel
GH->>WF: Tag pushed (docs-vX.Y.Z)
WF->>WF: parse-docs-tag
par Parallel build & validation
WF->>CA: build action
CA->>CA: checkout & npm run build
WF->>CA: lint action
CA->>CA: checkout & npm run lint
WF->>CA: test action
CA->>CA: checkout, setup-env, npx playwright install, npm run test
and
Note over WF: Wait for all jobs
end
WF->>CA: deploy-docs action (production)
CA->>CA: setup-env
CA->>CA: deploy-vercel (production)
CA->>Vercel: vercel pull --production, vercel build, vercel deploy --prebuilt
Vercel-->>CA: production deployment complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/setup-env/action.yaml (1)
6-8:repo-tokeninput is declared but never used.The
repo-tokeninput is marked as required but is not referenced anywhere in this action's steps. This could confuse users and suggests incomplete implementation or dead code.Either remove the unused input or document its intended purpose:
inputs: - repo-token: - description: 'Token to access the repo' - required: true
🤖 Fix all issues with AI agents
In @.github/actions/build/action.yaml:
- Around line 31-41: The cache save step "Save build outputs to cache" currently
uses the conditional "if: always()", which can persist failed or partial build
artifacts; change that condition to only run on success (e.g., remove the "if:
always()" line or replace it with a success-only condition such as "if:
success()") so actions/cache/save@v4 only saves artifacts from successful
builds.
In @.github/actions/deploy-vercel/action.yaml:
- Around line 29-36: The vercel pull step is missing VERCEL_ORG_ID and
VERCEL_PROJECT_ID which causes interactive prompts; add new inputs (e.g.,
inputs.vercel-org-id and inputs.vercel-project-id) to the action, then export
them as environment variables (VERCEL_ORG_ID and VERCEL_PROJECT_ID) and include
that env block on the "Pull environment variables from Vercel" step where
`vercel pull` is run; also add the same env block to the subsequent build and
deploy steps so `vercel` commands (build/deploy) use the provided org and
project IDs.
- Around line 40-45: The run block currently injects inputs directly into the
shell (vercel build ${{ inputs.build-args }}) which allows command injection;
fix by quoting and/or validating inputs and apply same change to deploy-args:
change invocations to use quoted expansions like vercel build "${{
inputs.build-args }}" (and vercel deploy "${{ inputs.deploy-args }}") so the
input is passed as a single argument rather than interpreted by the shell, and
optionally add a simple whitelist/regex check on inputs (alphanumeric, dashes,
spaces, equals, and known flags) before calling vercel to reject any suspicious
characters.
🧹 Nitpick comments (12)
.github/actions/setup-env/action.yaml (1)
18-25: Cache key uses commit SHA - cache won't be shared across commits.Using
github.shaas the cache key means each commit creates a new cache entry and never restores from previous commits. While this ensures exact reproducibility, it sacrifices the speed benefits of caching across commits with unchanged dependencies.Consider using package-lock.json hash for better cache reuse
- key: ${{ runner.os }}-install-${{ github.sha }} + key: ${{ runner.os }}-install-${{ hashFiles('**/package-lock.json') }}This allows cache hits when dependencies haven't changed, even across different commits.
.github/actions/lint/action.yaml (1)
9-12: Consider defaultingfetch-depthto1for lint action.Linting typically doesn't require git history. Using
fetch-depth: 1(shallow clone) would speed up the checkout step. The current default of0fetches the full history.fetch-depth: description: 'Git fetch depth for checkout' required: false - default: '0' + default: '1'.github/actions/test/action.yaml (2)
38-42: Consider caching Playwright browsers.Playwright browser installation is time-consuming. While the README states "Playwright browsers: Cached by the Playwright installer," this action doesn't configure Playwright's cache path or use GitHub Actions cache for browsers.
Add Playwright browser caching
+ - name: Get Playwright version + id: playwright-version + shell: bash + run: echo "version=$(npm ls `@playwright/test` --json | jq -r '.dependencies["@playwright/test"].version // .devDependencies["@playwright/test"].version')" >> $GITHUB_OUTPUT + + - name: Cache Playwright browsers + uses: actions/cache@v4 + with: + path: ~/.cache/ms-playwright + key: ${{ runner.os }}-playwright-${{ steps.playwright-version.outputs.version }} + - name: Install Playwright Browsers shell: bash run: npx playwright install --with-deps env: CI: true
9-12: Consider defaultingfetch-depthto1for test action.Similar to the lint action, tests typically don't require full git history. A shallow clone would speed up checkout.
fetch-depth: description: 'Git fetch depth for checkout' required: false - default: '0' + default: '1'.github/actions/deploy-docs/action.yaml (1)
34-37: Redundant checkout when action is called from workflows.This composite action includes a checkout step, but looking at the calling workflows (e.g.,
deploy-docs.yamllines 90-94,pull-request.yamllines 115-119), they already perform a checkout before invoking this action. This results in double checkouts.Consider either:
- Removing the checkout from this action and documenting that callers must checkout first, or
- Removing the checkout from calling workflows since this action handles it.
.github/actions/changed-packages/action.yaml (2)
48-50: Simplify command and add error handling.The current command has a useless echo wrapper and lacks explicit error handling. If
turbo build --dry-runfails, the workflow will continue with potentially malformed output.♻️ Proposed improvement
run: | # 1. We need the 'output' of a turbo dry-run to get a json with all affected packages # 2. The multi line json string is wrapped in EOF delimiters to make GHA happy # See: https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#multiline-strings # 3. We specify the .github/ folder as a dependency. If workflows changed, we use that to mean # everything has changes pending... to force redeploys + set -euo pipefail echo 'result<<CHANGESET_DELIMITER' >> $GITHUB_OUTPUT - echo "$(npx -y turbo build --dry-run=json --filter=...[$TURBO_REF_FILTER])" >> $GITHUB_OUTPUT + npx -y turbo build --dry-run=json --filter="...[$TURBO_REF_FILTER]" >> $GITHUB_OUTPUT echo 'CHANGESET_DELIMITER' >> $GITHUB_OUTPUT
26-31: Redundant checkout - same issue as deploy-docs action.This action performs its own checkout, but the calling workflow (
pull-request.yamllines 43-47) also performs a checkout before invoking this action. This is redundant and wastes CI time..github/workflows/release.yaml (1)
41-51: Double checkout pattern repeated across all jobs.Each job (build, lint, test, publish-packages) performs a shallow checkout (
fetch-depth: 1) just to access the action file, then the composite action performs another checkout withfetch-depth: '0'. This pattern is repeated 4 times.While this works, it's inefficient. Consider documenting this as an intentional pattern or restructuring the actions to avoid redundant checkouts.
.github/workflows/deploy-docs.yaml (2)
25-37: Unusedversionoutput fromparse-docs-tagjob.The
parse-docs-tagjob extracts and outputs the version from the tag, but this output is never referenced by any downstream job. Thedeploy-docsjob depends onparse-docs-tagbut doesn't useneeds.parse-docs-tag.outputs.version.Either:
- Remove this job if version extraction isn't needed, or
- Use the version output (e.g., for tagging the deployment, logging, or passing to build args).
♻️ Example: Use version in deploy step
- name: Deploy Docs to Vercel Production uses: ./.github/actions/deploy-docs timeout-minutes: 20 with: repo-token: ${{ secrets.GITHUB_TOKEN }} vercel-token: ${{ secrets.VERCEL_ACCESS_TOKEN }} vercel-project-id: ${{ secrets.VERCEL_DOCS_PROJECT_ID }} environment: production fetch-depth: '1' - build-args: '--prod' + build-args: '--prod --build-env DOCS_VERSION=${{ needs.parse-docs-tag.outputs.version }}' deploy-args: '--prod'
86-106: The workflow-level VERCEL_PROJECT_ID is redundant.The workflow sets
VERCEL_PROJECT_IDin itsenvblock (line 13), but thedeploy-docsjob doesn't use it directly. Instead, the job explicitly passesvercel-project-idas an input to the action (line 102), which then sets it again as an environment variable (action.yaml line 46). Both reference the same secret value, so there's no conflict, but the workflow-level env variable is unnecessary. Remove line 13 to simplify..github/workflows/pull-request.yaml (2)
16-18: Consider addingreopenedtrigger type.The workflow only triggers on
openedandsynchronizeevents. If a PR is closed and reopened, the quality checks won't run automatically. Consider addingreopenedto the types list.♻️ Proposed change
on: pull_request: - types: [opened, synchronize] + types: [opened, synchronize, reopened]
128-131: Empty string inputs could be omitted.Since
build-argsanddeploy-argshave default values of''in the action definition, explicitly passing empty strings is redundant.♻️ Simplified version
with: repo-token: ${{ secrets.GITHUB_TOKEN }} vercel-token: ${{ secrets.VERCEL_ACCESS_TOKEN }} vercel-project-id: ${{ secrets.VERCEL_DOCS_PROJECT_ID }} environment: preview fetch-depth: '0' - build-args: '' - deploy-args: ''
| run: | | ||
| if [ -n "${{ inputs.build-args }}" ]; then | ||
| vercel build ${{ inputs.build-args }} | ||
| else | ||
| vercel build | ||
| fi |
There was a problem hiding this comment.
Potential shell injection via build-args input.
The build-args and deploy-args inputs are directly interpolated into shell commands. If a caller passes malicious content (e.g., ; rm -rf /), it could execute arbitrary commands.
While this is a composite action called from trusted workflows, it's good practice to validate or quote inputs properly.
Safer alternative using environment variables
- name: Build project locally
shell: bash
+ env:
+ BUILD_ARGS: ${{ inputs.build-args }}
run: |
- if [ -n "${{ inputs.build-args }}" ]; then
- vercel build ${{ inputs.build-args }}
+ if [ -n "$BUILD_ARGS" ]; then
+ vercel build $BUILD_ARGS
else
vercel build
fiApply the same pattern to deploy-args.
🤖 Prompt for AI Agents
In @.github/actions/deploy-vercel/action.yaml around lines 40 - 45, The run
block currently injects inputs directly into the shell (vercel build ${{
inputs.build-args }}) which allows command injection; fix by quoting and/or
validating inputs and apply same change to deploy-args: change invocations to
use quoted expansions like vercel build "${{ inputs.build-args }}" (and vercel
deploy "${{ inputs.deploy-args }}") so the input is passed as a single argument
rather than interpreted by the shell, and optionally add a simple
whitelist/regex check on inputs (alphanumeric, dashes, spaces, equals, and known
flags) before calling vercel to reject any suspicious characters.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/README.md:
- Around line 139-151: Remove the duplicate "### `changed-packages` (Action)"
section from the Workflows area of the README: delete the entire header and its
Inputs/Outputs/Usage paragraph (the block starting at "### `changed-packages`
(Action)") so only the canonical documentation under the Composite Actions
section remains; ensure no other references to this copied block remain
elsewhere in the file.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.github/README.md:
- Around line 283-288: The Caching section's nested list items are indented 4
spaces which triggers markdownlint MD007; edit the nested bullet lines under
"npm dependencies" and "Build outputs" so each nested line is indented by 2
spaces (not 4) and keep the same bullet text/structure—update the lines
containing "`- Cache key: {os}-install-{commit-sha}`", "`- Only installs if
cache miss occurs`", "`- Cache key: {os}-build-{commit-sha}`", and "`- Restored
by `lint` and `test` jobs`" to use 2-space indentation.
1 participant
What does this PR do?
docsapp sinceapi-harmonization/frontendapps will never be deployed directly from this main repository (there is a separate repo for the demo app)Summary by CodeRabbit
Documentation
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.