Repository navigation
fix(ci): rebuild service images when their workspace packages change - #171
MarkAlex1234 wants to merge 1 commit into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (21)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughBackend and frontend Docker workflow path filters now include additional shared package directories. Existing path filters remain. ChangesShared-package workflow triggers
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to No concrete merge-blocking risk was identified in the workflow trigger updates. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change broadens existing build and deployment triggers without changing permissions or pull-request publication gates. No introduced vulnerability was established, but secret-access policy and downstream rollout recovery could not be confirmed. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Hardening Proposals
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| - 'packages/bus/**' | ||
| - 'packages/cache/**' | ||
| - 'packages/code-samples/**' | ||
| - 'packages/db/**' |
There was a problem hiding this comment.
A fork PR that changes only packages/db now starts this workflow. Fork PRs do not receive the Docker Hub secrets, but docker/login-action runs before the build and needs them. The check fails before it can build the image. Skip login on PR runs that do not push an image.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| - 'packages/bus/**' | ||
| - 'packages/cache/**' | ||
| - 'packages/code-samples/**' | ||
| - 'packages/db/**' |
There was a problem hiding this comment.
Two close pushes to main that change packages/db now start overlapping builds of this service. Both write the same latest tag without a run-order guard. If the older build finishes last, it replaces the newer image and its deploy request can leave the service running old package code. Run each service’s pushes in order or deploy by image digest.
1 participant
Summary
Each
be-*/fe-*Docker workflow builds withcontext: ., and the images copy/app/packages(e.g.apps/backend/api-key/Dockerfile). But most workflows'paths:only list the app's own directory. So a change merged tomainthat touches only a shared package (packages/db,packages/auth,packages/bus,packages/cache, …) doesn't rebuild or push any service image, and the published images keep running the old package code until something else in each app changes. This PR adds each app's transitive workspace dependencies underpackages/to both itspushandpull_requestpath lists. Nothing else in the workflows changes.The lists come from each app's
package.jsonworkspace:*dependencies, followed transitively. For example,be-api-keydepends on@reloop/db,cache,auth,busandcode-samples, andauthpulls indnsandwebhook-events.be-inbound,be-smtpandbe-spamwere already complete and are untouched.Linked issue
None. This is a small CI fix (path filters only), so I skipped opening an issue first; happy to open one if you prefer.
Type of change
fix— bug fixchore— tooling, CI, dependenciesAffected areas
.github/workflows/be-*.yml(16 files),.github/workflows/fe-*.yml(5 files)How tested
on.*.paths).packages/*its app (transitively) depends on.Not covered on purpose:
bun.lock/ rootpackage.json. Adding them would rebuild every image on any dependency bump.fe-dashboard.ymlalready does this, so say if you want it everywhere.Trade-off: shared packages like
db/authsit under most services, so a change there will now rebuild most images. That is the correct result, but it is more CI than today. If that becomes a cost problem, the alternative is a single workflow that computes the affected services and builds them as a matrix. Disclosure: I maintain a GitHub Action that does that, dynamic-monorepo, and its dependency graph is how I found these gaps. This PR doesn't use it.Deploy / breaking notes
None.
Checklist
bun run checkpasses (lint + format): not applicable, YAML-only change outside the linted sourcesSummary by CodeRabbit
The PR is not safe to merge until fork PRs can build without Docker Hub secrets and service pushes cannot publish older images last.
Findings
Summary
The PR adds transitive workspace-package paths to service image workflows so package-only changes start rebuilds.
MarkAlex1234 acknowledged that shared-package changes will rebuild many images and described the extra CI work as the intended trade-off.
Reviews (1) · Last reviewed commit: "fix(ci): rebuild service images when the..."