chore(CI): improve workflow merging steps and parallelizing image build #37352
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
WalkthroughThis PR restructures the CI/CD pipeline from platform-centric to service-oriented orchestration. Changes include renaming inputs and artifacts (platform → service, turbo-build → packages-build), implementing per-service matrix builds with source-hash caching, updating Docker compose references, removing turbo task dependencies, and consolidating artifact handling across workflows. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant RV as release-versions
participant PBS as Packaging Build (per-service)
participant SB as Service Build
participant DB as build-docker
participant DH as DockerHub Publish
GH->>RV: Trigger
RV->>RV: Compute source-hash
RV-->>GH: Output: services[], source-hash
Note over PBS: Matrix per service
GH->>PBS: Build packages (cached by source-hash)
PBS-->>GH: Upload packages-build artifact
Note over SB: Per-service orchestration
GH->>SB: service param + packages artifact
SB->>SB: Restore packages-build
SB->>SB: Unpack packages tarball
SB->>DB: Invoke with service: 'rocketchat'
DB->>DB: docker compose build (service)
DB-->>GH: Image built & tagged
GH->>DH: Publish all service images
DH-->>GH: Published to DockerHub
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37352 +/- ##
===========================================
+ Coverage 66.88% 67.01% +0.13%
===========================================
Files 3411 3418 +7
Lines 117029 117751 +722
Branches 21490 21555 +65
===========================================
+ Hits 78274 78915 +641
- Misses 36071 36159 +88
+ Partials 2684 2677 -7
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
rodrigok
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
docker-compose-ci.yml (1)
7-7: Inconsistent Dockerfile path format between CI and local environments.Line 7 uses an absolute path with
${GITHUB_WORKSPACE}, whiledocker-compose-local.yml(line 7) uses a relative path. This inconsistency could cause issues if the compose file is used in different execution contexts. Consider aligning both to use relative paths for portability.Additionally, verify that
DOCKER_TAGenvironment variable is consistently set in all workflows that invoke this compose file.Also applies to: 11-11
.github/workflows/ci.yml (1)
72-72: Consider deriving services list from docker-compose-ci.yml.The services list (line 72) is hardcoded as a long string. While explicit, this creates a maintenance burden—if services are added or removed in
docker-compose-ci.yml, this list must be manually updated or the build will miss services or fail. Consider dynamically extracting the service names fromdocker-compose-ci.ymlusingjqto keep the two in sync.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.github/actions/build-docker/action.yml(2 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(14 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (2/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (1/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 8.2 (4/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (2/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (4/4)
- GitHub Check: 🔨 Test UI (CE) / MongoDB 5.0 (1/4)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (5/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (1/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (4/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (2/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (5/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (3/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 8.2 [legacy watchers] (3/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (1/5)
- GitHub Check: 🔨 Test UI (EE) / MongoDB 5.0 (2/5)
- GitHub Check: 🔨 Test API (EE) / MongoDB 5.0 (1/1)
🔇 Additional comments (6)
.github/actions/build-docker/action.yml (2)
48-72: Verify artifact name and path consistency between workflows.The action now expects a
packages-buildartifact with fileRocketChat-packages-build.tar.gz(lines 51–52, 57), and conditionally restores a separatebuildartifact only for therocketchatservice (lines 59–64). Confirm thatci.ymlproduces these artifacts with exactly these names and file paths to prevent runtime failures.
79-79: Service-based build and push logic is sound.The change from a platform-based approach to service-based (using
${{ inputs.service }}) enables parallel builds for different services. The conditional cleanup (line 88) ensures only rocketchat's temporary build directory is removed, which is appropriate since other services don't use the/tmp/buildpath.Also applies to: 85-85, 88-88
.github/workflows/ci.yml (4)
180-190: Artifact creation and upload is properly structured.The new
packages-buildartifact is created, archived asRocketChat-packages-build.tar.gz, and uploaded with appropriate retention (lines 180–190). Ensure the glob patterns in line 183 capture all necessary build outputs for downstream builds.
233-233: Service matrix properly parameterizes the build job.The build-gh-docker job now accepts a service matrix derived from the services list (line 233), enabling parallel builds of all 8 services. The
serviceinput is correctly passed to the build-docker action (line 247).
283-283: Test job dependencies correctly updated to the new build job.Test jobs (test-api, test-ui, test-api-ee, test-ui-ee, test-ui-ee-watcher) now depend on
build-gh-dockerinstead of the previousbuild-gh-docker-coverage. This aligns with the renamed build job (line 221) and ensures tests run after Docker images are built.Also applies to: 299-299, 324-324, 343-343, 370-370
495-495: Docker image publishing refactored for per-service delivery.The docker-image-publish job now uses a service matrix (line 495) to publish each service's image independently, enabling parallelization. The dynamic image-name extraction via docker-compose config (lines 521–526) is a solid approach, though verify the jq fix (see prior comment) is applied. Service-specific naming (rocketchat → rocket.chat, others as-is) correctly handles the edge case.
Also applies to: 533-578
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
72-72: Hardcoded services list requires manual maintenance.The services list is hardcoded as a comma-separated string in the workflow output. When new services are added to the build matrix, this list must be updated manually. Consider whether a more dynamic approach (e.g., reading from a configuration file, environment variable, or docker-compose-ci.yml) would reduce maintenance burden.
Current list:
['rocketchat','authorization-service','account-service','ddp-streamer-service','presence-service','stream-hub-service','queue-worker-service','omnichannel-transcript-service']This is not a blocking issue, but worth noting for future refactors.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.github/actions/build-docker/action.yml(2 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(14 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose-ci.yml
- docker-compose-local.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (8)
.github/workflows/ci-test-e2e.yml (1)
147-147: Correct service parameter addition.Adding
service: 'rocketchat'aligns with the refactored build-docker action interface. This is necessary for the service-driven approach..github/workflows/ci.yml (6)
39-47: Good addition: Github Info diagnostic step.The new diagnostic output (GITHUB_ACTION, GITHUB_ACTOR, refs, event details, and event payload) improves CI observability for debugging workflow issues. This is helpful for understanding context at the start of the pipeline.
221-247: Approve build-gh-docker job structure and service-matrix approach.The new per-service build matrix correctly parallelizes Docker image builds. Dependencies and conditional logic are sound:
- Matrix consumes services from release-versions output ✓
- Conditional build step ensures builds only for appropriate branches/events ✓
- Service parameter correctly passed to build-docker action ✓
283-283: Correct dependency updates for test jobs.Test jobs now depend on
build-gh-dockermatrix. This ensures all services are built before tests begin, which is correct given that tests need the rocketchat container.Also applies to: 299-299, 324-324, 343-343, 370-370
518-526: ✅ Past critical issue resolved: jq variable reference is now correct.The previous review flagged that line 522 referenced an undefined
$servicevariable. This has been corrected to use${{ matrix.service }}, ensuring the GitHub Actions workflow renderer injects the correct matrix value before the shell executes the jq command.GH_IMAGE_NAMEwill now be properly populated for per-service image naming.
533-537: Approve special handling for rocketchat image naming.The conditional logic correctly handles the fact that the service name is
rocketchatbut the published image name isrocket.chat(with dot). This naming distinction is properly abstracted and prevents publish failures.
542-578: Approve per-service tagging and publishing logic.The refactored tagging and push logic correctly:
- Builds a tags array with ref-based tag, release-based tags (for tagged releases), and commit SHA
- Properly qualifies remote repository with lowercase-repo output
- Uses docker tag and push commands for per-service distribution
.github/actions/build-docker/action.yml (1)
59-72: Searching for docker-compose files and Dockerfile build context configuration in the codebase:Based on web search findings, paths in Dockerfile COPY commands must be relative paths and will be interpreted as relative to the source of the build context. Common Docker build errors occur when files are placed in /tmp and the build context doesn't include them.
The verification script did not find
docker-compose-ci.ymlor explicit/tmp/buildreferences in Dockerfiles, which is concerning. Given that GitHub Actions examples showcontext: .being required for Docker builds, and the fact that the packages-build extraction uses the repository root (.) while meteor build uses/tmp/build, this path mismatch needs clarification.Extract meteor build to repository root for consistency and accessibility to docker build context.
The meteor build extracted to
/tmp/builddiffers from packages-build handling (.). Without confirming that docker-compose-ci.yml explicitly sets/tmp/buildas the build context—and given that no such configuration was found in the codebase—files extracted to/tmp/buildmay not be accessible during the Docker build phase.Extract to the repository root to match packages-build behavior and ensure Docker build context includes the files:
- name: Unpack meteor build if: inputs.service == 'rocketchat' shell: bash run: | - cd /tmp/build - tar xzf Rocket.Chat.tar.gz - rm Rocket.Chat.tar.gz + tar xzf /tmp/build/Rocket.Chat.tar.gz -C . + rm /tmp/build/Rocket.Chat.tar.gz
rodrigok
force-pushed
the
improve/ci
branch
2 times, most recently
from
3981709 to
2d6cf7b
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
488-578: Critical issue: Invalid JSON format in services output breaks matrix population.Line 72 outputs services with single quotes instead of double quotes:
echo "services=['rocketchat','authorization-service',...]" >> $GITHUB_OUTPUTThis is invalid JSON. The
fromJson()function in the docker-image-publish job (line 495) will fail to parse single-quoted JSON. The matrix will remain empty, preventing any Docker images from being published.Fix required: Change line 72 to use double quotes:
echo "services=[\"rocketchat\",\"authorization-service\",\"account-service\",\"ddp-streamer-service\",\"presence-service\",\"stream-hub-service\",\"queue-worker-service\",\"omnichannel-transcript-service\"]" >> $GITHUB_OUTPUT
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
34-34: Consider documenting the service list and verifying against docker-compose-ci.yml.The hardcoded services array (line 72) enables explicit, auditable service matrix configuration but requires manual updates if services are added/removed. The Github Info diagnostic block (lines 39–47) is helpful for troubleshooting.
Consider adding a comment above line 72 documenting that this list must be kept in sync with the services defined in
docker-compose-ci.yml. Optionally, a follow-up task could explore dynamic service discovery (e.g., parsing docker-compose-ci.yml at build time), though this adds complexity.Also applies to: 39-47, 72-72
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.github/actions/build-docker/action.yml(2 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(14 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose-local.yml
- docker-compose-ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
.github/actions/build-docker/action.yml (2)
74-90: Conditional logic for service-specific build and publish is well-structured.The action correctly gates meteor build restoration (lines 60, 67) and cleanup (line 93) on
service == 'rocketchat', enabling efficient per-service builds. The dynamic image extraction via jq (line 86) and direct tagging/pushing (lines 88, 90) support the new parallelized service matrix.
48-72: Artifact extraction paths are correctly aligned with downstream docker-compose-ci.yml usage.Verification confirms:
packages-buildextracted to repo root (line 57) — correctly accessed by services usingcontext: .(authorization-service, account-service, presence-service, etc.)build(meteor) extracted to/tmp/build(lines 70–72) — correctly referenced by rocketchat service withcontext: /tmp/build(docker-compose-ci.yml line 8)No path inconsistencies detected. The configuration properly segregates per-service build contexts.
.github/workflows/ci-test-e2e.yml (1)
80-80: Simplification of test job naming and explicit service invocation.The job name is now clearer and the explicit
service: 'rocketchat'parameter (line 147) makes the intent explicit. Removal of rc-dockerfile and rc-docker-tag inputs (per summary) correctly decouples the test workflow from per-platform configuration, deferring to the centralized service-driven docker-compose-ci.yml.Also applies to: 147-147
.github/workflows/ci.yml (2)
180-203: Package archival and build dependency chain is well-structured.The packages-build job correctly archives build artifacts (using
-oiflags confirmed in previous review as necessary for gitignored files) and the build job appropriately depends on it. The retention-days setting (5 days) balances cost with artifact availability. Job naming clarification ("📦 Meteor Build") improves workflow readability.
221-247: Parallelized per-service docker builds improve CI throughput.The new
build-gh-dockerjob introduces a service matrix (line 233) that builds each service in parallel using the refactored build-docker action. All downstream test jobs now depend on this job (lines 283, 299, 324, 343, 370), ensuring builds are parallelized before tests start.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.github/actions/build-docker/action.yml(2 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(14 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose-local.yml
- .github/workflows/ci-test-e2e.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (7)
.github/workflows/ci.yml (5)
180-190: Approve packages-build artifact workflow.The new packages-build archive and artifact upload steps (lines 180–190) correctly capture build outputs and enable downstream job reuse. The git ls-files command with
-oiand--exclude-standardflags correctly includes gitignored dist artifacts while excluding node_modules, .meteor, .deno-cache, .turbo, and livechat directories. Per prior review confirmation, this approach is sound.
221-233: Approve service-centric build matrix refactoring.The build-gh-docker job now uses a service matrix (line 233:
fromJson(needs.release-versions.outputs.services)) instead of a platform-based matrix, enabling true parallelization of per-service image builds. The job correctly passes the service to the build-docker action (line 247) and maintains proper dependency ordering vianeeds: [build, release-versions].
518-526: Approve docker image name extraction logic.Line 522 correctly extracts the image name from docker-compose-ci.yml using the matrix service as the jq selector. The reference to
${{ matrix.service }}is now correct (per prior review fix), and the logic properly accounts for the rocketchat service mapping to a different image name (rocket.chat) in docker-compose-ci.yml.
533-578: Approve per-service DockerHub publishing logic.The docker-image-publish job now handles per-service publishing with conditional image naming (rocketchat → rocket.chat) and consistent tag logic across all services. The refactored tag array construction (lines 542–564) and multi-tag push (line 578) correctly implement the new per-service approach while maintaining compatibility with release and commit-hash tagging strategies.
283-283: No changes needed — GitHub Actions correctly handles matrix job dependencies.GitHub Actions by design ensures that when a job uses
needs: <job-id>, the dependent job waits for every matrix run of that job to complete. The workflow correctly specifiesneeds: [checks, build-gh-docker, release-versions]for test jobs, and sincebuild-gh-dockerdoes not define any outputs that are consumed by dependent jobs, there is no risk of nondeterministic behavior. The dependency chain resolves as intended.docker-compose-ci.yml (1)
7-7: Environment variables are properly configured.The
build-gh-dockerjob in ci.yml (lines 226-228) setsDOCKER_TAGandLOWERCASE_REPOSITORYas job-level environment variables, which are automatically inherited by all shell steps in the composite action.GITHUB_WORKSPACEis a GitHub Actions built-in variable always available in shell steps. The docker compose command (action.yml line 98) runs with these variables in scope..github/actions/build-docker/action.yml (1)
59-72: Artifact dependency is properly enforced through workflow structure; review comment concern is incorrect.The workflow dependency chain guarantees the
buildartifact exists when needed. All ci-test-e2e.yml calls in ci.yml explicitly depend onbuild-gh-docker, which depends on thebuildjob that uploads the meteor build artifact. Thebuild-dockeraction correctly gates the meteor build restore/unpack to onlyrocketchatservice via the conditionalif: inputs.service == 'rocketchat', ensuring the step only runs when the artifact is actually needed. The workflow structure prevents any scenario where the artifact could be missing when the download step runs.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
72-72: 🔴 Critical: Fix services output to valid JSON format.Line 72 outputs a shell-style array with single quotes, but
fromJson()on lines 233 and 495 expects valid JSON. This mismatch will cause the matrix definitions to fail at runtime.Apply this diff to output valid JSON:
- echo "services=['rocketchat','authorization-service','account-service','ddp-streamer-service','presence-service','stream-hub-service','queue-worker-service','omnichannel-transcript-service']" >> $GITHUB_OUTPUT + echo "services=[\"rocketchat\",\"authorization-service\",\"account-service\",\"ddp-streamer-service\",\"presence-service\",\"stream-hub-service\",\"queue-worker-service\",\"omnichannel-transcript-service\"]" >> $GITHUB_OUTPUTAlternatively, construct it programmatically to ensure proper escaping.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
39-47: ✅ Github Info diagnostics step is helpful.The new step logs workflow context (event name, actor, refs, event path) which aids in debugging and observability. This is a good addition for operational visibility.
Consider adding
GITHUB_WORKFLOWandGITHUB_JOBto the output for even richer context tracing.
488-578: ✅ Docker image publish job correctly refactored for per-service flow.The
docker-image-publishjob now uses a service matrix (line 495) and derives per-service image names fromdocker-compose-ci.yml(line 522). The logic correctly handles the rocketchat special case (service name vs. image name on line 533-537) and applies consistent tagging (commit SHA, release tags). Sparse checkout is appropriate.Lines 561-566: The
COMMIT_SHAand tags array construction is effective, though consider extracting this into a separate step or helper for reusability if future jobs need identical tag logic.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
.github/actions/build-docker/action.yml(2 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(14 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- docker-compose-local.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
docker-compose-ci.yml (1)
7-7: ✅ Consistent service-centric image naming and tagging.The changes cleanly align with the per-service matrix approach: each service now has an explicit Dockerfile reference, uses the shared
${DOCKER_TAG}variable, and follows the consistent image naming patternghcr.io/${LOWERCASE_REPOSITORY}/[service-name]:${DOCKER_TAG}. Variables are correctly dereferenced.Also applies to: 11-11, 46-46, 63-63, 81-81, 98-98, 121-121, 139-139, 156-156
.github/workflows/ci-test-e2e.yml (1)
67-68: ✅ Clean workflow service-centric alignment.The removal of
rc-dockerfileandrc-docker-taginputs is correct; the workflow now relies on the sharedDOCKER_TAGenv variable sourced from the build action. Hardcodingservice: 'rocketchat'in the build-docker step is appropriate for E2E testing.Also applies to: 147-147
.github/actions/build-docker/action.yml (2)
85-91: ✅ Image publishing logic correctly fixed.The variable references on lines 89 and 91 are properly dereferenced with
${IMAGE_NO_TAG}. The dynamic image name derivation fromdocker-compose-ci.ymlcorrectly handles the rocketchat naming exception (service name vs. image name). The publish flow is sound.
48-72: ✅ Artifact restoration and build unpacking correctly sequenced.The new flow unpacks packages-build first (to root), then conditionally unpacks the meteor-specific build tarball to
/tmp/buildonly whenservice == 'rocketchat'. This supports per-service parallel builds while maintaining the rocketchat-specific build artifact handling..github/workflows/ci.yml (2)
180-190: ✅ Packages build archival correctly configured.The
git ls-files -oicommand is intentionally configured to capture gitignored build artifacts (dist directories) as confirmed in prior review discussion. The tar archival and artifact upload are properly configured with appropriate retention.
221-247: ✅ Service-centric build job properly structured.The new
build-gh-dockerjob uses a per-service matrix (line 233) derived from the services output, enabling parallel per-service Docker image builds. The job correctly passesmatrix.serviceto the build-docker action and maintains proper job dependencies.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
521-612: Confirm docker-image-publish depends on build-gh-docker completion.The
docker-image-publishjob (line 524) declaresneeds: [deploy, release-versions]but does not depend onbuild-gh-docker. Since this job pulls Docker images built bybuild-gh-docker, add that job to the needs list to ensure the images are available before publishing.docker-image-publish: name: 🚀 Publish Docker Images (DockerHub) runs-on: ubuntu-24.04 - needs: [deploy, release-versions] + needs: [deploy, release-versions, build-gh-docker]
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
87-87: Critical: Fix invalid JSON format in services output for fromJson() parsing.Line 87 uses shell-style array syntax with single quotes (
['rocketchat',...']),which is not valid JSON. Lines 266 and 528 callfromJson(needs.release-versions.outputs.services)to populate the service matrix, but this will fail to parse the current output format.- echo "services=['rocketchat','authorization-service','account-service','ddp-streamer-service','presence-service','stream-hub-service','queue-worker-service','omnichannel-transcript-service']" >> $GITHUB_OUTPUT + echo "services=[\"rocketchat\",\"authorization-service\",\"account-service\",\"ddp-streamer-service\",\"presence-service\",\"stream-hub-service\",\"queue-worker-service\",\"omnichannel-transcript-service\"]" >> $GITHUB_OUTPUT
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
50-56: Remove or uncomment the commented sparse-checkout configuration.Lines 50–56 contain commented-out sparse-checkout options. If they're not needed, remove them to keep the workflow clean. If they should be used, uncomment and document why sparse-checkout is beneficial here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.github/actions/build-docker/action.yml(2 hunks).github/actions/meteor-build/action.yml(8 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(15 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose-ci.yml
- docker-compose-local.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
166-166: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (5)
.github/actions/build-docker/action.yml (1)
48-57: Service-based Docker build action looks solid.The refactoring from platform to service is clean. Previous variable reference issues (lines 89–91) are now correctly fixed with proper
${IMAGE_NO_TAG}and${IMAGE}dereferencing. Artifact restoration and unpacking logic is in place, and conditionals correctly gate restore/unpack/cleanup onservice == 'rocketchat'.Also applies to: 79-79, 81-93
.github/workflows/ci-test-e2e.yml (1)
147-147: Service parameter correctly added to build-docker call.The e2e workflow now uses the service-centric approach. Removed platform-specific inputs (rc-dockerfile, rc-docker-tag) and correctly passes
service: 'rocketchat'to the build-docker action..github/workflows/ci.yml (3)
254-280: Verify service matrix is properly populated after fixing JSON format.Once line 87 is fixed to output valid JSON, the per-service matrix on line 266 will correctly expand. Confirm all 8 services from the matrix are intended for Docker image building and publishing.
After fixing line 87, verify the workflow matrix expands correctly by checking GitHub Actions logs to ensure all services are iterated:
rocketchat,authorization-service,account-service,ddp-streamer-service,presence-service,stream-hub-service,queue-worker-service,omnichannel-transcript-service.
210-214: I'll need to investigate further to understand whether the patterns are correct. Let me check the .gitignore rules and validate the git command syntax:I need to investigate the
-oiflag combination more carefully and check the .gitignore to understand the actual behavior:Based on my verification, I've found a critical issue with the review comment's requested verification approach:
The git command syntax is valid, but verification is inconclusive on a clean branch.
The git source code shows that "-i must be used with either -o or -c", and the combined
-oiflags mean: when showing "other" files, show only those matched by an exclude pattern. The 0 results here are expected on a clean PR branch without build artifacts—the command cannot verify proper archival without actualdist/*directories and built files present.The
:()pathspec syntax is valid per modern git versions, but the complex patterns used (positional inclusions like*/dist/*combined with:(exclude)rules) make comprehensive verification impossible without:
- An actual build run that generates the artifacts
- Real dist directories to test against
- Understanding what files should actually be archived
Since this is checking CI workflow behavior, the review comment should be rewritten to acknowledge this practical limitation.
Run the git ls-files command during build with actual artifacts to verify file capture.
The command syntax is valid but should be tested in the actual CI build context where dist directories and build outputs are present. The 0 result on a clean branch is expected and does not indicate an error. To properly verify that all expected build artifacts are captured, this command needs to be executed after the build phase generates files—not on a fresh repository.
165-176: The secondary verification claim is factually incorrect.The upgrade recommendation from actions/cache@v3 to v4 is valid—v4 is the current recommended major version. Line 166 correctly identifies
actions/cache@v3needing an upgrade.However, the review's request to "verify that other
actions/cache@v3usages in the file (lines 62, 70, 79) are also updated" is inaccurate. Lines 62, 70, and 79 do not containactions/cacheactions at all; they contain unrelated shell commands (tar, echo, and NODE_VERSION assignment). A full search of the file reveals only twoactions/cacheusages:
- Line 166:
actions/cache@v3(the primary issue)- Line 195:
actions/cache@v4(already upgraded)The core upgrade recommendation at line 166 remains sound, but remove the incorrect reference to lines 62, 70, 79.
Likely an incorrect or invalid review comment.
rodrigok
force-pushed
the
improve/ci
branch
3 times, most recently
from
774daae to
ff340b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/actions/meteor-build/action.yml (1)
125-125: ✅ Logical operator correctly placed in Reset Meteor conditional.The && operator is now present between cache-hit check and reset-meteor input. The condition syntax is valid.
.github/workflows/ci.yml (1)
66-70: ✅ Source hash output syntax is correct.No unmatched quotes; both echo statements are properly formed.
🧹 Nitpick comments (2)
.github/actions/build-docker/action.yml (1)
87-93: Remove unusedIMAGE_NO_TAGvariable.Line 89 computes
IMAGE_NO_TAGby stripping the tag fromIMAGE, but this variable is never used. Line 91 tags directly from${IMAGE}(the full image reference including tag). Either remove the dead variable or clarify the intended logic.IMAGE=$(docker compose -f docker-compose-ci.yml config --format json 2>/dev/null | jq -r --arg s "${{ inputs.service }}" '.services[$s].image') - IMAGE_NO_TAG=$(echo "$IMAGE" | sed 's/:.*$//') - docker tag ${IMAGE} ${IMAGE}-gha-run-${{ github.run_id }} docker push --all-tags ${IMAGE}docker-compose-ci.yml (1)
7-7: ExportGITHUB_WORKSPACEexplicitly in build-docker action before docker-compose invocation.The
docker-compose-ci.ymluses${GITHUB_WORKSPACE}for the Dockerfile path (line 7), but the build-docker action does not explicitly export it before runningdocker compose(unlikeDENO_VERSION). WhileDOCKER_TAGandLOWERCASE_REPOSITORYare properly set at the job level inci.yml(lines 260–261) and inherited by the action,GITHUB_WORKSPACE—though available as a GitHub Actions runner variable—should be explicitly exported in.github/actions/build-docker/action.ymlfor consistency and robustness.Update the "Build Docker images" step to:
export DENO_VERSION="${{ inputs.deno-version }}" export GITHUB_WORKSPACE="${GITHUB_WORKSPACE}" docker compose -f docker-compose-ci.yml build ${{ inputs.service }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.github/actions/build-docker/action.yml(2 hunks).github/actions/meteor-build/action.yml(8 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(15 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose-local.yml
- .github/workflows/ci-test-e2e.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
166-166: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (6)
.github/actions/meteor-build/action.yml (1)
23-26: ✅ Cache key strategy is sound.The
source-hashinput enables deterministic caching keyed to source changes, with proper fallback restore-keys for partial hits. Cache gates are correctly applied to prevent redundant build steps.Also applies to: 52-60
.github/actions/build-docker/action.yml (1)
48-72: ✅ Artifact restoration and unpacking logic is correct.Artifact naming and paths are consistent across download and extraction steps. Service-specific conditions properly gate meteor build restoration to the rocketchat service only.
.github/workflows/ci.yml (4)
251-251: ✅ Source hash correctly passed to meteor-build action.
263-280: ⏸️ Blocked by services output format issue (line 87).The matrix strategy correctly uses
fromJson()to consume the services list, but will fail if the output on line 87 remains in shell-style format. Once line 87 is corrected to output valid JSON, this matrix strategy will work as intended.
521-612: ✅ Per-service publishing workflow structure is sound.The refactored docker-image-publish job correctly:
- Uses per-service matrix (dependent on line 87 fix)
- Sparsely checks out only docker-compose-ci.yml
- Derives image names dynamically per service
- Constructs and pushes multiple tags
Verify that the
jqextraction on line 555 correctly handles all service image references from docker-compose-ci.yml across different naming conventions (e.g.,rocket.chatvs service namerocketchat).
166-166: The review comment contains inaccurate claims and should be disregarded.The verification reveals:
Incorrect file reference:
build-docker/action.ymlline 53 does not contain a cache action—it contains configuration for a build directory path.Incomplete scope: The review misses multiple additional
actions/cache@v3usages that actually exist:
meteor-build/action.yml: lines 62, 71, 80 (in addition to line 53)setup-node/action.yml: line 39Incorrect deprecation status: v3 is not deprecated or outdated. Both v3 and v4 remain officially supported by GitHub Actions as of October 2025 (the deprecation deadline for v1/v2 was February 1, 2025). Upgrading to v4 is a best-practice recommendation, not a critical requirement.
If a cache version standardization is desired, the scope should be updated to include all locations; however, the current review is based on flawed facts.
Likely an incorrect or invalid review comment.
rodrigok
force-pushed
the
improve/ci
branch
2 times, most recently
from
a535c42 to
fd6fd99
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
166-170: Update cache action to v4.The actionlint static analysis tool reports that
actions/cache@v3is outdated. Update to v4 for better compatibility with the latest GitHub Actions runtime.- - name: Cache build - uses: actions/cache@v3 + - name: Cache build + uses: actions/cache@v4 id: cache-build with:Based on static analysis hints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
.github/actions/build-docker/action.yml(3 hunks).github/actions/meteor-build/action.yml(9 hunks).github/workflows/ci-test-e2e.yml(2 hunks).github/workflows/ci.yml(15 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- docker-compose-local.yml
- docker-compose-ci.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
166-166: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
292-292: input "type" is not defined in action "Meteor Docker" defined at "./.github/actions/build-docker". available inputs are "CR_PAT", "CR_USER", "NPM_TOKEN", "deno-version", "node-version", "publish-image", "service", "setup", "turbo-cache", "tyoe"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (8)
.github/workflows/ci-test-e2e.yml (1)
68-68: ✅ Service-based alignment looks good.The removal of platform-specific inputs (rc-dockerfile, rc-docker-tag) and addition of the service input cleanly aligns with the per-service architecture. The DOCKER_TAG derivation with "-cov" suffix is appropriate for coverage-enabled builds.
Also applies to: 80-80, 147-147
.github/actions/meteor-build/action.yml (1)
52-160: ✅ Effective caching strategy implementation.The addition of source-hash-based caching with proper cache-hit guards across multiple steps (lines 63, 72, 81, 90, 111, 131, 146, 151) prevents redundant builds and leverages the new source-hash input effectively. The turbo caching upgrade to v1.8 and input consolidation (coverage → type) improve maintainability and flexibility.
.github/actions/build-docker/action.yml (1)
52-103: ✅ Per-service artifact flow is well-structured.The refactoring from platform-centric to service-centric artifact handling (packages-build vs. meteor build separation, per-service unpack with conditional gating on service == rocketchat) enables better parallelization and cleaner dependency management. The derivation of image names from docker-compose-ci.yml (line 92) provides a single source of truth.
.github/workflows/ci.yml (5)
233-257: ✅ Build matrix restructuring for per-type parallelization is sound.The new matrix-based approach (lines 237-240) correctly parallelizes production and coverage builds, enabling concurrent execution and better resource utilization. The source-hash input enables effective cache-keying based on source code changes.
259-293: Verify build-docker service matrix and type consistency.After fixing the critical issues above (services JSON format and type assignment), verify that the build-docker job receives the correct matrix variables and that the per-service docker compose build (line 83 in build-docker action) works correctly for all services in the matrix (rocketchat + microservices).
Ensure that all services in the output array (line 87) have corresponding entries in docker-compose-ci.yml and that each service entry has a properly defined
imagefield for the jq query to extract (line 92 in build-docker action).
328-415: ✅ Test job dependencies correctly updated.The test jobs (test-api, test-ui, test-api-ee, test-ui-ee, test-ui-ee-watcher) now properly depend on the build-gh-docker job (lines 328, 344, 369, 388, 415), ensuring docker images are available before tests start. Dependency chain is clear and logical.
533-623: ✅ Docker Hub publishing per-service architecture is well-designed.The per-service matrix in docker-image-publish (line 540) correctly iterates over all services. The service-specific image naming logic (lines 578-582) handles the rocketchat → rocket.chat image name mapping. Tag array assembly and docker push logic is properly structured.
68-70: The review comment is incorrect.The code at lines 68-70 in
.github/workflows/ci.ymlcontains no quote mismatches or syntax errors. Both echo statements use proper unquoted variable expansion:echo hash=${SOURCE_HASH}. The quotes are matched (there are none to mismatch), and the variables are correctly echoed. The syntax is valid for both YAML and shell contexts.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/actions/build-docker/action.yml (1)
1-2: Add missing action description.The action metadata is missing a top-level
descriptionfield. While not a functional issue, actionlint expects this for proper action documentation.name: 'Meteor Docker' +description: 'Build and publish Docker image for Rocket.Chat Meteor service' inputs:.github/workflows/ci.yml (1)
245-252: Consider removing unused turbo-build artifact.The turbo-build artifact (lines 245-252) may be unused now that the workflow has migrated to packages-build and livechat-build artifacts. Verify whether any downstream jobs depend on it, then consider removing it to reduce storage overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
.github/actions/build-docker/action.yml(3 hunks).github/actions/meteor-build/action.yml(9 hunks).github/workflows/ci-code-check.yml(1 hunks).github/workflows/ci-deploy-gh-pages.yml(1 hunks).github/workflows/ci-test-e2e.yml(3 hunks).github/workflows/ci-test-storybook.yml(1 hunks).github/workflows/ci-test-unit.yml(1 hunks).github/workflows/ci.yml(14 hunks).github/workflows/new-release.yml(1 hunks).github/workflows/pr-update-description.yml(1 hunks).github/workflows/publish-release.yml(1 hunks).github/workflows/release-candidate.yml(1 hunks)docker-compose-ci.yml(1 hunks)docker-compose-local.yml(1 hunks)turbo.json(0 hunks)
💤 Files with no reviewable changes (1)
- turbo.json
🚧 Files skipped from review as they are similar to previous changes (8)
- .github/workflows/pr-update-description.yml
- .github/workflows/publish-release.yml
- .github/workflows/ci-test-storybook.yml
- .github/workflows/ci-deploy-gh-pages.yml
- docker-compose-local.yml
- .github/workflows/ci-test-e2e.yml
- .github/actions/meteor-build/action.yml
- docker-compose-ci.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
305-305: description is required in metadata of "Meteor Docker" action at "/home/jailuser/git/.github/actions/build-docker/action.yml"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (15)
.github/workflows/release-candidate.yml (1)
24-24: ✓ LGTMThe turbo cache step is properly disabled, aligning with the PR's shift to artifact-based package caching. This change enables downstream jobs to use pre-built packages from the
packages-buildartifact..github/workflows/new-release.yml (1)
43-43: ✓ LGTMConsistent turbo cache deprecation aligned with the PR's migration to artifact-based caching.
.github/workflows/ci-code-check.yml (1)
44-66: ✓ LGTMArtifact restoration and unpacking correctly implements the new caching strategy. The transition from turbo cache to pre-built artifact artifacts (
packages-build,livechat-build) enables parallelization and faster recovery. Extraction to workspace root is correct..github/workflows/ci-test-unit.yml (1)
48-70: ✓ LGTMConsistent with
ci-code-check.yml. Artifact restoration and unpacking follow the same pattern, enabling efficient test execution against pre-built packages..github/actions/build-docker/action.yml (3)
16-34: ✓ LGTMInput refactoring is clean: renaming
platform→serviceand addingtype(production/coverage) properly implements the per-service build strategy. Both inputs are well-documented.
48-72: ✓ LGTMArtifact restoration and unpacking correctly implements the new strategy. The conditional unpack (lines 59-72) appropriately limits meteor build restoration to rocketchat service only.
79-93: ✓ LGTMDocker build and publish logic correctly derives service configuration from
docker-compose-ci.yml. The image extraction and tagging operations are sound. Assumes service is defined in the compose file (validated at CI runtime)..github/workflows/ci.yml (8)
34-35: ✓ LGTMNew outputs (
services,source-hash) correctly enable per-service matrix and source-based cache invalidation.
58-74: ✓ LGTMSource archival and hashing implementation correctly creates a reproducible archive (with fixed ownership, timestamps, sorting) and outputs its SHA256 hash for downstream cache validation. The echo statements have been properly fixed.
169-178: ✓ LGTMCache strategy correctly uses
source-hashas the cache key, enabling smart invalidation. Build is skipped if cache hits, significantly improving CI performance for unchanged sources.
219-243: ✓ LGTMBuild archival and artifact uploads are correct. The
git ls-files -oicommands properly capture gitignored build artifacts while excluding unnecessary directories. Artifacts are uploaded with appropriate retention (5 days).
254-279: ✓ LGTMBuild job correctly parallelizes on
type(production/coverage) and passessource-hashfor artifact validation and tracing. The dependency onpackages-buildjob is properly declared.
281-316:⚠️ Blocked by critical line 91 issue.The per-service matrix (line 293) depends on valid JSON output from the
servicesoutput (line 91). Once the JSON syntax is fixed, this job will correctly parallelize Docker builds across all services with conditional coverage builds.Verify DOCKER_TAG_SUFFIX_ROCKETCHAT usage:
Line 308 defines
DOCKER_TAG_SUFFIX_ROCKETCHATconditionally, but it's not passed to thebuild-dockeraction (lines 310-316). Verify whether this should be passed as an input or if it's intended for use elsewhere in the workflow.[depends_on: line 91 fix]
557-595:⚠️ Blocked by critical line 91 issue.The per-service matrix (line 564) depends on valid JSON output from
services. Sparse checkout is efficient, and image extraction from compose config is correct.[depends_on: line 91 fix]
600-648: ✓ LGTMDocker image publishing logic correctly handles per-service tagging and pushing. Conditional service naming (rocketchat vs. others) and release-based tag selection are properly implemented.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/meteor-build/action.yml (1)
131-175: 🚨 Critical: Artifact restore steps must be guarded by cache-hit condition.Lines 131–140 unconditionally download and unpack the packages-build artifact, but this artifact only exists when the upstream
packages-buildjob runs (i.e., when cache misses). Whensteps.cache-build.outputs.cache-hit == 'true', the artifact is unavailable and the download will fail.Wrap both the download and unpack steps in a cache-hit guard identical to the build and other conditional steps.
- name: Restore packages build uses: actions/download-artifact@v4 + if: steps.cache-build.outputs.cache-hit != 'true' with: name: packages-build path: /tmp - name: Unpack packages build shell: bash + if: steps.cache-build.outputs.cache-hit != 'true' run: | tar -xzf /tmp/RocketChat-packages-build.tar.gz -C .
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/actions/meteor-build/action.yml(9 hunks).github/workflows/ci-code-check.yml(1 hunks).github/workflows/ci-test-unit.yml(1 hunks).github/workflows/ci.yml(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-test-unit.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
291-291: description is required in metadata of "Meteor Docker" action at "/home/jailuser/git/.github/actions/build-docker/action.yml"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (9)
.github/workflows/ci-code-check.yml (1)
46-56: LGTM – Artifact restoration and unpacking correctly integrated.The changes align with the new packages-build caching strategy. Artifact download path (
/tmp) and extraction to repository root are correct..github/workflows/ci.yml (6)
40-48: LGTM – Telemetry step appropriate for operational visibility.The Github Info step provides environment context for debugging and correlating CI runs.
58-74: LGTM – Source hash computation correct and reproducible.The tar creation uses deterministic flags (sorted, fixed ownership/mtime) and excludes transient directories, ensuring stable cache keys across runs.
169-230: LGTM – Source-hash caching and artifact archival properly implemented.The per-run source-hash key provides deterministic caching; all build and archive steps are correctly guarded. Separation of packages-build archive and turbo-cache storage is appropriate.
241-302: LGTM – Per-service matrix and build restructuring properly implemented.The transition from
platformtotype(production/coverage) with per-service matrix enables parallel image builds. The include clause correctly restricts coverage builds to rocketchat only. The newDOCKER_TAG_SUFFIX_ROCKETCHATenvironment variable is prepared for coverage image differentiation.Verify that the
DOCKER_TAG_SUFFIX_ROCKETCHATenvironment variable (line 294) is consumed in.github/actions/build-docker/action.ymlto apply the coverage suffix to the rocketchat image tag. This should be applied whenmatrix.type == 'coverage'.
338-338: LGTM – Job dependencies correctly updated.All test jobs properly reference the renamed
build-gh-dockerjob instead of the olddocker-image-publishname, maintaining correct execution order.Also applies to: 354-354, 379-379, 398-398, 425-425
543-633: LGTM – Per-service Docker publishing workflow well-structured.Per-service matrix enables parallel image publishing. Sparse checkout optimization, jq-based image name extraction from docker-compose config, and service-specific routing (rocketchat vs. other services) are correct. Tagging logic properly handles release versions, commit SHAs, and latest tags. The bash array operations for tag management are sound.
.github/actions/meteor-build/action.yml (2)
4-26: LGTM – Inputs correctly refactored for per-type builds and source-hash caching.The transition from
coverage(boolean) totype(string) with default 'production' provides clearer semantics. Newsource-hashinput enables deterministic cache key derivation in the meteor build action.
32-93: LGTM – Caching strategy and conditional execution properly implemented.The cache step uses source-hash for deterministic key derivation; all build and tool installation steps are correctly guarded by cache-hit condition. The rharkor/caching-for-turbo step runs unconditionally (line 123), which is appropriate since turbo maintains its own cache independent of the RC build cache.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
291-291: Add missing description field to build-docker action metadata.Static analysis flagged that the build-docker action lacks a description in its metadata. Update
./.github/actions/build-docker/action.ymlto include adescriptionfield at the top level for better documentation and actionlint compliance.Add this to
./.github/actions/build-docker/action.yml:name: 'Build Docker Image' description: 'Builds and pushes Docker images per service' # ... rest of action.yml
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/actions/meteor-build/action.yml(9 hunks).github/workflows/ci-code-check.yml(1 hunks).github/workflows/ci-test-unit.yml(1 hunks).github/workflows/ci.yml(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/actions/meteor-build/action.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
291-291: description is required in metadata of "Meteor Docker" action at "/home/jailuser/git/.github/actions/build-docker/action.yml"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 📦 Meteor Build (production)
- GitHub Check: 📦 Meteor Build (coverage)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (8)
.github/workflows/ci-test-unit.yml (1)
50-59: ✓ Artifact handling updated correctly for new build packaging approach.The refactored artifact retrieval and unpacking steps align with the stated CI improvements:
- Artifact naming (
packages-build) and tarball extraction pattern are consistent.- Step ordering is correct: artifact download → unpack → tests.
- Tar command syntax is valid.
To ensure robustness across the broader workflow refactor, verify that:
- The upstream build job reliably creates the
packages-buildartifact with the expected tarball structure.- The extraction to the workspace root (
.) doesn't conflict with existing project structure.- The
download-artifactaction behavior when an artifact is unavailable is acceptable for your workflow (typically fails the job).If you'd like, I can verify these dependencies by inspecting the related workflows mentioned in the AI summary (ci-test-e2e.yml, ci-code-check.yml, ci.yml).
.github/workflows/ci-code-check.yml (1)
46-55: ✓ Artifact restoration and unpacking align with per-service packaging changes.The replacement of turbo-build artifact restoration with packages-build and the subsequent unpack step are correctly positioned and named. This supports the downstream per-service workflows.
.github/workflows/ci.yml (6)
58-74: Verify source hash computation produces stable, reproducible results.The source tarball is created with deterministic options (
--sort=name --owner=0 --group=0 --mtime='1970-01-01'), which is excellent for cache key stability. Confirm thathashFiles()on/tmp/RocketChat-source.tarworks correctly when called in the same job step context after the tar is written.Can you confirm that the tar file is successfully created and hashFiles produces consistent output across multiple runs with identical source?
588-634: ✓ Per-service Docker image publishing with multi-tag logic is well-structured.The conditional image naming (rocketchat vs other services), multi-tag accumulation, and loop-based tagging approach are correct. The per-service matrix enables parallel publishing across all services.
338-338: ✓ Dependency updates correctly reflect build-gh-docker job renaming.All test jobs (test-api, test-ui, test-api-ee, test-ui-ee, test-ui-ee-watcher) properly depend on the new
build-gh-dockerjob instead of the oldbuildjob. No missing or circular dependencies detected.Also applies to: 354-354, 379-379, 398-398, 425-425
291-302: Build-docker action correctly accepts bothserviceandtypeparameters.Verification confirms that
./.github/actions/build-docker/action.ymldefines both inputs:
service(optional, type: string, description: 'Container to build')type(optional, description: 'production or coverage', default: 'production')The parameters passed at lines 301–302 are properly supported by the action.
261-265: No issues found—meteor-build action properly accepts all parameters.The meteor-build action (
.github/actions/meteor-build/action.yml) defines all parameters passed at lines 261–265:type,source-hash,node-version, anddeno-version. Thetypeparameter is properly configured with the updated description "production or coverage" (default: production), confirming the prior refactoring fromcoveragetotypewas completed successfully.
279-285: No issues found—all matrix services are defined in docker-compose-ci.yml.All 8 services in the hardcoded list (rocketchat, authorization-service, account-service, ddp-streamer-service, presence-service, stream-hub-service, queue-worker-service, omnichannel-transcript-service) are present in docker-compose-ci.yml. The jq query at line 577 will execute successfully for each service.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/actions/meteor-build/action.yml (2)
131-140: Guard Restore and Unpack steps by cache-hit condition.The "Restore packages build" and "Unpack packages build" steps are unconditional, but the "Build Rocket.Chat" step (line 144) is guarded by
cache-hit != 'true'. This means the unpack steps run even when the cache hits and the build is skipped, wasting resources. These steps should only run when the build actually needs the packages.- name: Restore packages build uses: actions/download-artifact@v4 + if: steps.cache-build.outputs.cache-hit != 'true' with: name: packages-build path: /tmp - name: Unpack packages build shell: bash + if: steps.cache-build.outputs.cache-hit != 'true' run: | tar -xzf /tmp/RocketChat-packages-build.tar.gz -C .
169-175: 🚨 Critical: Store build artifact must be guarded by cache-hit condition.The "Prepare build" step (lines 162–167) creates
/tmp/Rocket.Chat.tar.gzonly when the cache misses (gated byif: steps.cache-build.outputs.cache-hit != 'true'). However, the "Store build" step attempts to upload this file unconditionally. When the cache hits, the tarball is not created, causing the artifact upload to fail.- name: Store build uses: actions/upload-artifact@v4 + if: steps.cache-build.outputs.cache-hit != 'true' with: name: build-${{ inputs.type }} path: /tmp/Rocket.Chat.tar.gz overwrite: true include-hidden-files: true
🧹 Nitpick comments (3)
.github/workflows/ci.yml (3)
40-48: Debug step can be removed or conditionally logged.The new "Github Info" debug step (lines 40–48) logs environment variables and event details for every workflow run. While useful for debugging, this adds noise to logs. Consider removing it or gating it to only run on pull requests or debugging branches.
51-56: Sparse-checkout commented out but not removed.Lines 51–56 have sparse-checkout configuration commented out instead of being removed. If this is intentional dead code, delete it; if it may be needed later, add a brief comment explaining why it's disabled.
240-265: Verify meteor-build action receives all required inputs.The build job now passes
source-hashandtypeinputs to the meteor-build action (lines 260–265). Ensure downstream workflows and local developers understand the new requiredsource-hashinput when calling this action independently. Consider documenting this in the action's README or a migration guide.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/actions/meteor-build/action.yml(9 hunks).github/workflows/ci-code-check.yml(1 hunks).github/workflows/ci-test-unit.yml(1 hunks).github/workflows/ci.yml(14 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci-code-check.yml
🧰 Additional context used
🪛 actionlint (1.7.8)
.github/workflows/ci.yml
291-291: description is required in metadata of "Meteor Docker" action at "/home/jailuser/git/.github/actions/build-docker/action.yml"
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
.github/workflows/ci.yml (3)
169-230: Approve cache-restore and archive-upload pattern.The cache-build step (lines 169–177) correctly uses
restore-keysto restore/tmp/RocketChat-packages-build.tar.gzon cache hit, making the unconditional archive and upload steps safe. When cache hits, the restored file is uploaded; when cache misses, the new archive is created and uploaded. This pattern is sound.
267-285: Approve per-service Docker build matrix with coverage include.The new build-gh-docker job (lines 267–285) uses a per-service matrix (derived from release-versions outputs) with a conditional coverage build for rocketchat. This enables parallelized per-service image builds and targeted coverage testing. Architecture is sound.
543-577: ✓ Verification complete: docker-compose-ci.yml is properly configured with all required services.The docker-compose-ci.yml file exists at the repository root and defines all 8 services referenced in the services matrix (line 91): rocketchat, authorization-service, account-service, ddp-streamer-service, presence-service, stream-hub-service, queue-worker-service, and omnichannel-transcript-service. Each service has a valid
imageproperty, so the jq extraction at line 577 will succeed for all services in the docker-image-publish matrix.
sampaiodiego
added
the
stat: QA assured
dionisio-bot
bot
added
the
stat: ready to merge
https://rocketchat.atlassian.net/browse/ARCH-1853
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit