chore(ci): fix image size report #37631
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughA new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ 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 |
rodrigok
force-pushed
the
chore/ci-fix-image-size
branch
from
ef1b20a to
becba69
Compare
📦 Docker Image Size Report📈 Changes
📊 Historical Trend---
config:
theme: "dark"
xyChart:
width: 900
height: 400
---
xychart
title "Image Size Evolution by Service (Last 30 Days + This PR)"
x-axis ["11/15 22:28", "11/16 01:28", "11/17 23:50", "11/18 22:53", "11/19 23:02", "11/21 16:49", "11/24 17:34", "11/27 19:12", "11/27 19:26 (PR)"]
y-axis "Size (GB)" 0 --> 0.5
line "account-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "authorization-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "ddp-streamer-service" [0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12, 0.12]
line "omnichannel-transcript-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13]
line "presence-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
line "queue-worker-service" [0.14, 0.14, 0.14, 0.14, 0.14, 0.13, 0.13, 0.13, 0.13]
line "rocketchat" [0.36, 0.36, 0.35, 0.35, 0.35, 0.34, 0.34, 0.35, 0.35]
line "stream-hub-service" [0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11, 0.11]
Statistics (last 8 days):
ℹ️ About this reportThis report compares Docker image sizes from this build against the
Updated: Thu, 27 Nov 2025 19:26:52 GMT |
rodrigok
force-pushed
the
chore/ci-fix-image-size
branch
from
becba69 to
8db29dc
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/actions/docker-image-size-tracker/action.yml (1)
251-270: Missing error handling for git push failures.The git push command (line 267) could fail due to authentication errors (invalid/expired ci-pat, insufficient permissions, network issues). There is no error checking or handling for these failures.
Add error handling to ensure failures are properly logged and the step fails fast if push operations cannot be completed:
cd /tmp/history-worktree git add "history/${timestamp}.json" git commit -m "Add measurement for ${timestamp} (${commit_sha:0:7})" + set -e # Exit on first error git push origin image-size-history + if [[ $? -ne 0 ]]; then + echo "Failed to push image size history. Check CI_PAT permissions." + exit 1 + fi cd -
🧹 Nitpick comments (1)
.github/actions/docker-image-size-tracker/action.yml (1)
207-208: Redundant git config—will be overridden later.The git user config set here (github-actions[bot]) will be overridden by new config in the "Save current measurement to history" step (lines 264–266). This is wasteful and creates confusion about which identity is actually used.
Consider moving all git config setup to the step where it's actually needed, or consolidating configurations.
📜 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 (2)
.github/actions/docker-image-size-tracker/action.yml(2 hunks).github/workflows/ci.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). (7)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔨 Test Storybook / Test Storybook
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 📦 Meteor Build (coverage)
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (3)
.github/workflows/ci.yml (1)
453-461: Verify CI_PAT secret availability and permissions.The
ci-patinput is now required by the action. Ensure that theCI_PATsecret is properly defined in the repository and has adequate permissions (content: write) for pushing to the image-size-history branch.Additionally, consider how this behaves in fork pull requests where repository secrets are typically unavailable. The step may fail silently or noisily depending on error handling in the action.
.github/actions/docker-image-size-tracker/action.yml (2)
9-11: Input definition looks good.The
ci-patinput is properly declared as required. Ensure the action documentation or README is updated to explain this breaking change.
264-266: Let me fix the commands and investigate further:
<function_calls>
#!/bin/bashCheck the action.yml file directly
echo "=== Full context of action.yml (lines 250-275) ==="
sed -n '250,275p' .github/actions/docker-image-size-tracker/action.ymlecho ""
echo "=== Search for ci-pat usage ==="
rg -n "ci-pat|CI_PAT" -C 2echo ""
echo "=== Search for git config patterns ==="
rg -n "git config" -C 3echo ""
echo "=== Check SECURITY.md ==="
head -100 SECURITY.md
</tool_call></blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| git commit -m "Add measurement for ${timestamp} (${commit_sha:0:7})" | ||
| git config --global url.https://${{ inputs.github-token }}@github.com/.insteadOf https://github.com/ | ||
| git config --global user.email "ci@rocket.chat" | ||
| git config --global user.name "rocketchat-ci[bot]" | ||
| git config --global url.https://${{ inputs.ci-pat }}@github.com/.insteadOf https://github.com/ | ||
| git push origin image-size-history |
There was a problem hiding this comment.
Critical: Git config must be set before git commit and push.
The git user configuration (lines 264–265) is set after the git commit (line 263), meaning the commit is attributed to the wrong identity. While the URL config for push (line 266) comes before git push (line 267)—which is correct—the overall ordering is problematic:
- Commit is made with the identity from lines 207–208 (github-actions[bot])
- Identity is then changed to rocketchat-ci[bot]
- Push uses the new identity, creating an inconsistency in git history
Move all git config setup (lines 264–266) to before line 262 (git add), so that both commit and push use the correct identity consistently.
cd /tmp/history-worktree
+ git config --global user.email "ci@rocket.chat"
+ git config --global user.name "rocketchat-ci[bot]"
+ git config --global url.https://${{ inputs.ci-pat }}@github.com/.insteadOf https://github.com/
git add "history/${timestamp}.json"
git commit -m "Add measurement for ${timestamp} (${commit_sha:0:7})"
- git config --global user.email "ci@rocket.chat"
- git config --global user.name "rocketchat-ci[bot]"
- git config --global url.https://${{ inputs.ci-pat }}@github.com/.insteadOf https://github.com/
git push origin image-size-historyCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
.github/actions/docker-image-size-tracker/action.yml around lines 263 to 267:
the git user/email and URL config are set after the git commit, causing the
commit to be attributed to the wrong identity; move the three git config lines
(user.email, user.name, and url.insteadOf) to before the git add/commit sequence
(i.e., place them before line 262) so the configured identity is used for the
commit and the URL credential helper is in place prior to push; keep the git
commit and git push lines as-is after the moved config lines.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37631 +/- ##
===========================================
+ Coverage 68.81% 68.82% +0.01%
===========================================
Files 3361 3361
Lines 114277 114277
Branches 20618 20618
===========================================
+ Hits 78634 78648 +14
+ Misses 33550 33533 -17
- Partials 2093 2096 +3
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.