ci: split quality gate into parallel jobs#19
Conversation
Split the single sequential `quality` job into 4 independent parallel jobs (lint, type-check, test, build) for faster CI feedback. Each job is registered as a required status check on the main branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe GitHub Actions release workflow was restructured: the previous Changes
Sequence Diagram(s)sequenceDiagram
participant Developer as Developer
participant Workflow as "Release Workflow"
participant Setup as "setup-node Action"
participant Lint as "Lint Job"
participant TypeCheck as "Type-check Job"
participant Test as "Test Job"
participant Build as "Build Job"
participant Publish as "Publish Jobs"
rect rgba(200,230,201,0.5)
Developer->>Workflow: push / create release
end
rect rgba(187,222,251,0.5)
Workflow->>Setup: invoke composite setup (node, npm ci)
Setup-->>Lint: configured
Setup-->>TypeCheck: configured
Setup-->>Test: configured
Setup-->>Build: configured
end
rect rgba(255,224,178,0.5)
Lint->>Workflow: complete lint
TypeCheck->>Workflow: complete type-check
Test->>Workflow: complete tests
Build->>Workflow: complete build (env vars set)
end
rect rgba(255,205,210,0.5)
Workflow->>Publish: trigger publish_github & publish_docker (all dependencies met)
Publish-->>Developer: release artifacts published
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 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)
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.
🧹 Nitpick comments (2)
.github/workflows/release.yml (2)
13-55: Nice parallelization. Optional: extract shared setup into a reusable workflow.The four parallel jobs achieve the PR goal of faster feedback. The repeated setup steps (checkout, setup-node, npm ci) are the expected trade-off for parallelization.
For future maintainability, if setup complexity grows (e.g., adding more environment variables, caching strategies, or Node version updates), consider extracting the shared steps into a composite action or reusable workflow.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 13 - 55, The repeated setup steps in the parallel jobs (lint, type-check, test, build) should be extracted into a reusable workflow or composite action to avoid duplication; create a new reusable workflow or composite action that runs the common steps (actions/checkout@v4, actions/setup-node@v4 with node-version input and cache, and npm ci) and accept parameters for job-specific commands, then update each job (lint, type-check, test, build) to call that reusable workflow/action and only run the job-specific step (npm run lint, npm run type-check, npm run test -- --run, npm run build).
56-58: Consider addingNEXT_PUBLIC_WS_URLfor consistency with the Dockerfile.The Dockerfile sets both
NEXT_PUBLIC_API_URLandNEXT_PUBLIC_WS_URLas build args. For validation parity, consider adding the missing env var here.♻️ Suggested change
- run: npm run build env: NEXT_PUBLIC_API_URL: http://localhost:8000 + NEXT_PUBLIC_WS_URL: ws://localhost:8000🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 56 - 58, Add the missing NEXT_PUBLIC_WS_URL environment variable to the env block in the release workflow to match the Dockerfile's build args; specifically, alongside NEXT_PUBLIC_API_URL: http://localhost:8000 add NEXT_PUBLIC_WS_URL (e.g., ws://localhost:8000 or the equivalent value used in the Dockerfile) so CI validation and builds have parity with the Dockerfile's NEXT_PUBLIC_WS_URL setting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/release.yml:
- Around line 13-55: The repeated setup steps in the parallel jobs (lint,
type-check, test, build) should be extracted into a reusable workflow or
composite action to avoid duplication; create a new reusable workflow or
composite action that runs the common steps (actions/checkout@v4,
actions/setup-node@v4 with node-version input and cache, and npm ci) and accept
parameters for job-specific commands, then update each job (lint, type-check,
test, build) to call that reusable workflow/action and only run the job-specific
step (npm run lint, npm run type-check, npm run test -- --run, npm run build).
- Around line 56-58: Add the missing NEXT_PUBLIC_WS_URL environment variable to
the env block in the release workflow to match the Dockerfile's build args;
specifically, alongside NEXT_PUBLIC_API_URL: http://localhost:8000 add
NEXT_PUBLIC_WS_URL (e.g., ws://localhost:8000 or the equivalent value used in
the Dockerfile) so CI validation and builds have parity with the Dockerfile's
NEXT_PUBLIC_WS_URL setting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 39f83e5d-ed0e-451c-a0dc-e8c06d75c91f
📒 Files selected for processing (1)
.github/workflows/release.yml
- Create .github/actions/setup-node composite action to DRY up the repeated Node.js setup + npm ci steps across parallel CI jobs - Add NEXT_PUBLIC_WS_URL to the build job for parity with Dockerfile Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
84-86:⚠️ Potential issue | 🔴 CriticalRemove empty build-args or provide actual URLs—empty values will fail Zod URL validation at runtime.
The workflow passes
NEXT_PUBLIC_API_URL=andNEXT_PUBLIC_WS_URL=as empty strings to the Docker build. However,lib/env.tsvalidates these with Zod's.url()schema (line 12-14), which rejects empty strings. ThevalidateClientEnv()function uses the nullish coalesce operator (??), so the default value only applies when the variable is undefined or null—not when it's an empty string. This will cause the application to crash at startup with:Invalid client environment variables: {NEXT_PUBLIC_API_URL: ["Invalid url"]}.Either pass actual URLs (as documented in
RELEASING.mdandREADME.md) or omit these build-args entirely to rely on the default fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/release.yml around lines 84 - 86, The workflow is passing empty build-args NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WS_URL which fail Zod .url() validation in validateClientEnv() (lib/env.ts) because empty strings are not nullish; remove these empty build-args or supply real URLs as documented so the values are either undefined (allowing your defaults) or valid URLs; update the release.yml step to omit NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WS_URL entirely or replace them with proper URLs to prevent Invalid client environment variables errors at startup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 84-86: The workflow is passing empty build-args
NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WS_URL which fail Zod .url() validation in
validateClientEnv() (lib/env.ts) because empty strings are not nullish; remove
these empty build-args or supply real URLs as documented so the values are
either undefined (allowing your defaults) or valid URLs; update the release.yml
step to omit NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WS_URL entirely or replace them
with proper URLs to prevent Invalid client environment variables errors at
startup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7b1f9a7-9d29-44be-bd6d-d9ef7968066d
📒 Files selected for processing (2)
.github/actions/setup-node/action.yml.github/workflows/release.yml
- Remove empty build-args from Docker publish step (they caused Zod .url() validation to fail since empty strings aren't nullish) - Use || instead of ?? in validateClientEnv() so empty strings fall back to undefined, letting Zod .default() provide the fallback URL Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
qualityjob into 4 independent parallel jobs:lint,type-check,test,buildmainTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit