Added trigger_cd job and streamlined CI Docker builds#26546
Open
Added trigger_cd job and streamlined CI Docker builds#26546
Conversation
Contributor
WalkthroughThe changes remove the development Dockerfile and restructure the CI workflow to consolidate Docker builds. The standard Docker build job is removed while the production Docker build job is extended with admin artifact uploads (for non-fork PRs), additional output exports, and core image SHA tag extraction. A new trigger_cd job is introduced to dispatch CD workflow events with artifact metadata to an external repository, enabling production-focused builds with explicit downstream CD triggering. 🚥 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 |
rob-ghost
force-pushed
the
chore/wire-cd-dispatch
branch
2 times, most recently
from
5e8da5e to
d8ae1b5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26546 +/- ##
==========================================
+ Coverage 73.12% 73.14% +0.02%
==========================================
Files 1528 1528
Lines 120118 120154 +36
Branches 14491 14505 +14
==========================================
+ Hits 87832 87885 +53
+ Misses 31282 31264 -18
- Partials 1004 1005 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rob-ghost
force-pushed
the
chore/wire-cd-dispatch
branch
2 times, most recently
from
70c5800 to
de055c2
Compare
rob-ghost
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1051-1058: The core-sha-tag step can leave SHA_TAG empty and still
succeed, causing trigger_cd to dispatch with image_tag:""; update the
core-sha-tag step (id: core-sha-tag) to explicitly fail when SHA_TAG is empty by
exiting non-zero and writing a clear error to GITHUB_OUTPUT, or alternatively
add a guard in the trigger_cd job to check the output from core-sha-tag (the tag
variable produced via GITHUB_OUTPUT) and skip/abort dispatch if the tag is
empty; reference the SHA_TAG variable, core-sha-tag step id, and the image_tag
field used by trigger_cd when implementing the check.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.ymlDockerfile
💤 Files with no reviewable changes (1)
- Dockerfile
rob-ghost
force-pushed
the
chore/wire-cd-dispatch
branch
from
d10c3ff to
e91ac5b
Compare
Contributor
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/workflows/ci.yml (1)
1021-1029:⚠️ Potential issue | 🟠 MajorMake
loadconditional to avoid conflicting outputs.
Thepushandloadparameters indocker/build-push-action@v6use mutually exclusive output exporters. With the current setup, non-fork runs will fail when bothpush: trueandload: trueare active. Adjust the condition to load only when not pushing.Conditional load fix
- name: Build & push full image uses: docker/build-push-action@v6 with: context: /tmp/ghost-production file: Dockerfile.production target: full build-args: NODE_VERSION=${{ env.NODE_VERSION }} push: ${{ steps.strategy.outputs.should-push }} - load: true + load: ${{ steps.strategy.outputs.should-push == 'false' }} tags: ${{ steps.meta-full.outputs.tags }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1021 - 1029, The docker/build-push-action block currently sets both push and load which conflict; update the action (the block using docker/build-push-action@v6) so load is conditional and only true when not pushing by using the existing steps.strategy.outputs.should-push value (e.g., set load to the inverse of steps.strategy.outputs.should-push so load is true only when push is false).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 956-963: The upload step "Upload admin artifact for CD" (id:
upload-admin) currently uses actions/upload-artifact@v4 with path:
apps/admin/dist but lacks handling for missing files; update the step's with:
block to include if-no-files-found: error so the job fails fast when
apps/admin/dist is missing, preventing a silent successful step and downstream
CD using an empty artifact id.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1021-1029: The docker/build-push-action block currently sets both
push and load which conflict; update the action (the block using
docker/build-push-action@v6) so load is conditional and only true when not
pushing by using the existing steps.strategy.outputs.should-push value (e.g.,
set load to the inverse of steps.strategy.outputs.should-push so load is true
only when push is false).
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.ymlDockerfileghost/core/package.json
💤 Files with no reviewable changes (1)
- Dockerfile
rob-ghost
force-pushed
the
chore/wire-cd-dispatch
branch
from
e91ac5b to
374c210
Compare
- Added admin artifact upload step to job_docker_build_production, reusing the admin assets already built by `yarn build:production` rather than building them separately - Added trigger_cd job that dispatches to Ghost-Moya cd.yml via repository_dispatch with image tag, admin artifact ID, and PR number - trigger_cd depends only on job_setup + job_docker_build_production (not tests) so CD fires as soon as build artifacts are ready - All automated dispatches force dry_run=true until cd.yml is validated
The ghost-development image (job_docker_build) had no meaningful consumers — its only dependent was job_inspect_image which just reports size and layers. Removed both jobs and moved the inspect step directly into job_docker_build_production where the full image is already built. Changed full image build to always load locally so the inspect step can run docker inspect/history against it. Deleted the root Dockerfile which is no longer referenced by any workflow or compose file.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/ci.yml (2)
956-964: Previously flaggedif-no-files-found: erroris now in place — LGTM.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 956 - 964, The workflow step "Upload admin artifact for CD" (id: upload-admin, name: Upload admin artifact for CD) should keep the parameter if-no-files-found: error as currently set; no code changes required—ensure the actions/upload-artifact@v4 step retains name: admin-build-cd, path: apps/admin/dist, retention-days: 7 and if-no-files-found: error so missing artifacts will fail the job as intended.
1052-1060: Previously flagged hardening (exit 1on empty SHA tag) is now in place — LGTM.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1052 - 1060, The review note is a duplicate—no code change required in the "Extract core image SHA tag" step (id: core-sha-tag) that sets SHA_TAG; resolve or remove the duplicate review comment ([duplicate_comment]) in the PR thread so only the original LGTM message remains and do not modify the SHA_TAG extraction logic.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
1670-1670:peter-evans/repository-dispatch@v3is superseded by v4.v3.0.0 was released on 25 Jan and updated the runtime to Node.js 20. The current README now references
peter-evans/repository-dispatch@v4. Consider bumping to@v4to stay on the supported release.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 1670, The workflow is using the outdated action reference "uses: peter-evans/repository-dispatch@v3"; update this to the supported release by replacing the reference with "peter-evans/repository-dispatch@v4" in the CI workflow file, ensuring any input names or behavior differences are verified against the action's v4 README (adjust calling parameters if needed).
1021-1033: Consider splitting the push and load operations into separate steps to improve reliability on GitHub-hosted runners.The concurrent use of
push: trueandload: truein a single build step is technically supported by Buildx for single-platform builds, but is known to have intermittent failures on GitHub-hosted runners when using thedocker-containerdriver (see docker/build-push-action#1202). While this pattern may work most of the time, theload: trueoperation can fail out of the blue, making downstream steps likedocker inspectunreliable.To ensure consistent behavior, consider either:
- Splitting into two separate
docker/build-push-actionsteps — first to push, then to load from the pushed registry image for inspection.- Running a local registry service and pushing intermediate images to
localhost:5000/...instead (keepsdocker-containerdriver benefits).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1021 - 1033, The step "Build & push full image" currently sets both push: ${{ steps.strategy.outputs.should-push }} and load: true which can intermittently fail on GitHub-hosted runners; split this into two steps: keep the existing docker/build-push-action@v6 invocation but remove load: true so it only builds and pushes (preserve context, file, target, build-args, push, tags, labels, cache-from, cache-to), then add a second docker/build-push-action@v6 step that pulls or loads the already-pushed image (set push: false, remove push-related outputs, and use the same tags or ref to perform load: true or a registry pull) so inspection steps use the stable loaded image; reference the step name "Build & push full image", the docker/build-push-action@v6 action, and the push/load keys when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1654-1666: When is_main == 'true' the workflow currently emits
pr_number as an empty string; change the echo in that branch to emit a raw null
(unquoted) instead of an empty string so the resulting JSON has "pr_number":
null; specifically update the line that writes pr_number to $GITHUB_OUTPUT in
the is_main branch to output null (not "null" or ""), keep emitting dry_run=true
unchanged, and ensure the cd.yml contract (or downstream consumer) accepts a
nullable pr_number (or update cd.yml accordingly).
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 956-964: The workflow step "Upload admin artifact for CD" (id:
upload-admin, name: Upload admin artifact for CD) should keep the parameter
if-no-files-found: error as currently set; no code changes required—ensure the
actions/upload-artifact@v4 step retains name: admin-build-cd, path:
apps/admin/dist, retention-days: 7 and if-no-files-found: error so missing
artifacts will fail the job as intended.
- Around line 1052-1060: The review note is a duplicate—no code change required
in the "Extract core image SHA tag" step (id: core-sha-tag) that sets SHA_TAG;
resolve or remove the duplicate review comment ([duplicate_comment]) in the PR
thread so only the original LGTM message remains and do not modify the SHA_TAG
extraction logic.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 1670: The workflow is using the outdated action reference "uses:
peter-evans/repository-dispatch@v3"; update this to the supported release by
replacing the reference with "peter-evans/repository-dispatch@v4" in the CI
workflow file, ensuring any input names or behavior differences are verified
against the action's v4 README (adjust calling parameters if needed).
- Around line 1021-1033: The step "Build & push full image" currently sets both
push: ${{ steps.strategy.outputs.should-push }} and load: true which can
intermittently fail on GitHub-hosted runners; split this into two steps: keep
the existing docker/build-push-action@v6 invocation but remove load: true so it
only builds and pushes (preserve context, file, target, build-args, push, tags,
labels, cache-from, cache-to), then add a second docker/build-push-action@v6
step that pulls or loads the already-pushed image (set push: false, remove
push-related outputs, and use the same tags or ref to perform load: true or a
registry pull) so inspection steps use the stable loaded image; reference the
step name "Build & push full image", the docker/build-push-action@v6 action, and
the push/load keys when making the change.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/ci.ymlDockerfileghost/core/package.json
💤 Files with no reviewable changes (1)
- Dockerfile
rob-ghost
force-pushed
the
chore/wire-cd-dispatch
branch
from
374c210 to
8326340
Compare
Contributor
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/ci.yml (1)
1021-1033:⚠️ Potential issue | 🔴 CriticalGate
loadto non-push builds;pushandloadcannot be used together.
docker/build-push-action@v6usesload: true(type=docker exporter) andpush: true(type=registry exporter) as shorthand for mutually exclusive output modes. Using both simultaneously is unsupported. Whensteps.strategy.outputs.should-pushistrue, the action will fail.Apply the suggested fix to ensure
loadis only used during non-push builds:Fix
- load: true + load: ${{ steps.strategy.outputs.should-push == 'false' }}Alternatively, if both push and load are needed in a single build, refactor to use
outputswith multiple exporters (requires Buildx 0.13.0+), but that approach is incompatible with multi-platform builds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1021 - 1033, The job step "Build & push full image" sets both push: ${{ steps.strategy.outputs.should-push }} and load: true which are mutually exclusive; change the load input so it's only true when should-push is false (e.g., replace "load: true" with "load: ${{ steps.strategy.outputs.should-push != 'true' }}" or equivalent conditional) so the docker/build-push-action@v6 step does not attempt to use both push and load simultaneously.
♻️ Duplicate comments (1)
.github/workflows/ci.yml (1)
1652-1667:pr_numberis still emitted as an empty string on main pushes.This is the same issue previously flagged; if
cd.ymlexpectsnullor a number, the empty string may be problematic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1652 - 1667, The params step currently emits an empty string for pr_number on main pushes which can break downstream cd.yml; update the "Determine dispatch parameters" step (id: params) to emit a null value instead of an empty string for the main-branch branch case (i.e., replace echo "pr_number=" with echo "pr_number=null"), ensure the pull_request branch still emits the numeric value via pr_number=${{ github.event.pull_request.number }}, and keep outputs dry_run/skip semantics consistent so downstream consumers receive either a numeric pr_number or null, not an empty string.
🤖 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/ci.yml:
- Around line 1021-1033: The job step "Build & push full image" sets both push:
${{ steps.strategy.outputs.should-push }} and load: true which are mutually
exclusive; change the load input so it's only true when should-push is false
(e.g., replace "load: true" with "load: ${{ steps.strategy.outputs.should-push
!= 'true' }}" or equivalent conditional) so the docker/build-push-action@v6 step
does not attempt to use both push and load simultaneously.
---
Duplicate comments:
In @.github/workflows/ci.yml:
- Around line 1652-1667: The params step currently emits an empty string for
pr_number on main pushes which can break downstream cd.yml; update the
"Determine dispatch parameters" step (id: params) to emit a null value instead
of an empty string for the main-branch branch case (i.e., replace echo
"pr_number=" with echo "pr_number=null"), ensure the pull_request branch still
emits the numeric value via pr_number=${{ github.event.pull_request.number }},
and keep outputs dry_run/skip semantics consistent so downstream consumers
receive either a numeric pr_number or null, not an empty string.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/ci.ymlDockerfile
💤 Files with no reviewable changes (1)
- Dockerfile
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref https://linear.app/ghost/issue/BER-3294/
Commit 1: Added trigger_cd job and admin artifact upload to CI
Ghost CI builds production Docker images and admin assets but doesn't tell Ghost-Moya about them. The existing
canaryanddeploy_adminjobs useaurelien-baudet/workflow-dispatchto trigger Ghost-Moya's legacydeploy.ymlanddeploy-admin.ymlworkflows. The newcd.ymlpipeline in Ghost-Moya expects arepository_dispatchevent instead, with the core image tag and admin artifact details in the payload.Admin artifact upload is added as a step inside
job_docker_build_production— afteryarn build:productioncompletes, the built admin is uploaded as anadmin-build-cdartifact. No separate job needed since admin is already built as part of the production build.trigger_cddispatches aghost-artifacts-readyevent to Ghost-Moya with the core image SHA tag, admin artifact ID, and run metadata. It depends only onjob_setupandjob_docker_build_production— not on test completion. Tests run in parallel as a quality signal but don't gate the CD trigger; the environment's own smoke test in cd.yml is the deployment gate.dry_runis forced totrueon all automated dispatches until cd.yml is validated and ready to replace the existing canary deploy.Requires Ghost-Moya cd.yml to be on the default branch —
repository_dispatchonly triggers workflows on main. Done: #127, #132, #133, #134.Commit 2: Removed development Docker image build and folded image inspect inline
Removed
job_docker_build— the development image (ghost-developmentfrom rootDockerfile) had no meaningful consumers. Its only dependent wasjob_inspect_imagewhich just reports image size and layers to the step summary.Removed
job_inspect_imageas a separate job and moved the inspect step directly intojob_docker_build_production. The full image build now always setsload: trueso the image is available locally fordocker inspect/docker history.Deleted root
Dockerfile— the 228-line multi-stage development Dockerfile is no longer referenced anywhere. Local dev usesdocker/ghost-dev/Dockerfile(viacompose.dev.yaml), CI usesDockerfile.production, and E2E usese2e/Dockerfile.e2e.