Conversation
WalkthroughAdds a new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (6)📓 Common learnings📚 Learning: 2025-05-27T18:08:00.458ZApplied to files:
📚 Learning: 2025-05-27T18:08:00.458ZApplied to files:
📚 Learning: 2025-11-25T13:09:33.918ZApplied to files:
📚 Learning: 2025-11-25T14:28:50.351ZApplied to files:
📚 Learning: 2025-11-25T14:28:50.351ZApplied to files:
⏰ 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). (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docker/stripe/entrypoint.sh (1)
90-91: Add validation for GHOST_URL environment variable.The webhook forwarding URL uses
${GHOST_URL}without checking if it's set. If GHOST_URL is missing, the stripe listen command could fail with a confusing error message. While compose files always set this variable, defensive validation would improve robustness.Consider adding a validation check before the stripe listen command:
+# Check if GHOST_URL is set +if [ -z "${GHOST_URL:-}" ]; then + echo "ERROR: GHOST_URL is not set" + exit 1 +fi + # Start stripe listen in the background echo "Starting Stripe webhook listener forwarding to ${GHOST_URL}/members/webhooks/stripe/"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
compose.dev.yaml(2 hunks)compose.yml(1 hunks)docker/ghost-dev/entrypoint.sh(1 hunks)docker/stripe/entrypoint.sh(1 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use `yarn dev:forward` for hybrid Docker + host development with backend in Docker and frontend dev servers on host
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 24862
File: .github/workflows/ci-docker.yml:320-324
Timestamp: 2025-09-10T21:24:49.363Z
Learning: In GitHub Actions workflows using docker compose, the `docker compose pull` command works correctly with fork PRs even when some images (like the Ghost development image) are only loaded locally and not available in remote registries. The command doesn't fail when it encounters locally-loaded images during the pull process.
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.
Applied to files:
compose.dev.yamlcompose.yml
📚 Learning: 2025-05-27T18:08:00.458Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23546
File: compose.yml:58-59
Timestamp: 2025-05-27T18:08:00.458Z
Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.
Applied to files:
compose.dev.yaml
📚 Learning: 2025-11-25T13:09:33.918Z
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25485
File: compose.dev.yaml:0-0
Timestamp: 2025-11-25T13:09:33.918Z
Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.
Applied to files:
compose.dev.yamlcompose.yml
📚 Learning: 2025-12-01T08:42:35.320Z
Learnt from: jonatansberg
Repo: TryGhost/Ghost PR: 25552
File: e2e/helpers/environment/service-managers/dev-ghost-manager.ts:210-247
Timestamp: 2025-12-01T08:42:35.320Z
Learning: In e2e/helpers/environment/service-managers/dev-ghost-manager.ts, the hardcoded volume name 'ghost-dev_shared-config' at line 231 is intentional. E2E tests run under the 'ghost-dev-e2e' project namespace but deliberately mount the shared-config volume from the main 'ghost-dev' project to access Tinybird credentials created by yarn dev:forward. This is cross-project volume sharing by design.
Applied to files:
compose.dev.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Frontend dev servers and foundation libraries run on host machine during `yarn dev:forward`, while Ghost Core backend, MySQL, Redis, Mailpit, and Caddy run in Docker
Applied to files:
compose.dev.yaml
📚 Learning: 2025-10-07T12:19:15.174Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25031
File: ghost/core/test/utils/fixtures/config/defaults.json:68-80
Timestamp: 2025-10-07T12:19:15.174Z
Learning: In Ghost's configuration system (ghost/core/core/shared/config/), environment-specific config files (e.g., config.development.json, config.production.json) automatically fall back to values defined in defaults.json. It's only necessary to specify changed overrides on a per-env basis. Missing keys in env configs are not an issue—they're intentional and will use the default values.
Applied to files:
compose.dev.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:dev` to start Ghost in Docker with hot reload
Applied to files:
compose.dev.yaml
📚 Learning: 2025-02-06T10:30:53.440Z
Learnt from: allouis
Repo: TryGhost/Ghost PR: 22127
File: .github/scripts/release-apps.js:72-74
Timestamp: 2025-02-06T10:30:53.440Z
Learning: In the Ghost repository, the path to defaults.json is correctly structured as 'ghost/core/core/shared/config/defaults.json' with an intentional double 'core' in the path.
Applied to files:
compose.dev.yaml
📚 Learning: 2025-11-25T14:28:50.351Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional services
Applied to files:
compose.dev.yaml
🔇 Additional comments (3)
docker/ghost-dev/entrypoint.sh (1)
20-30: Well-implemented optional Stripe configuration.The webhook secret sourcing follows the established Tinybird pattern, with proper validation and graceful degradation when the config is missing. This maintains backward compatibility while enabling Stripe when the service is active.
compose.yml (1)
288-305: Stripe service configuration is well-structured.The GHOST_URL correctly references the
serverservice, environment variables follow consistent patterns with defaults, and the healthcheck appropriately validates webhook secret generation. The optional dependency from the server service is correct.compose.dev.yaml (1)
69-69: Add stripe dependency to ghost-dev service for consistency and reliability.The server service in compose.yml has a
stripedependency (required: false), but the ghost-dev service in compose.dev.yaml lacks this. Without the dependency, ghost-dev may start before stripe has created/mnt/shared-config/.env.stripe, causing the Stripe configuration to fail silently on startup.Add a stripe dependency to the ghost-dev service to match the pattern in compose.yml:
ghost-dev: build: context: ./ dockerfile: docker/ghost-dev/Dockerfile container_name: ghost-dev working_dir: /home/ghost/ghost/core command: ["yarn", "dev"] volumes: - ./ghost:/home/ghost/ghost # ... other volumes ... - shared-config:/mnt/shared-config:ro environment: NODE_ENV: development # ... env vars ... depends_on: mysql: condition: service_healthy redis: condition: service_healthy mailpit: condition: service_healthy + stripe: + condition: service_healthy + required: false healthcheck: # ... healthcheck ...Based on learnings, services that depend on shared configuration should properly order their startup dependencies.
Also applies to: 123-140
⛔ Skipped due to learnings
Learnt from: cmraible Repo: TryGhost/Ghost PR: 23546 File: compose.yml:58-59 Timestamp: 2025-05-27T18:08:00.458Z Learning: Services that are dependencies for both Ghost Docker Compose profiles (`ghost` and `split`) need to include both profiles in their `profiles` configuration to ensure they start correctly under either profile.Learnt from: jonatansberg Repo: TryGhost/Ghost PR: 25485 File: compose.dev.yaml:0-0 Timestamp: 2025-11-25T13:09:33.918Z Learning: In the Ghost Docker Compose development setup (compose.dev.yaml), the analytics service (ghost/traffic-analytics:1.0.20) requires `platform: linux/amd64` to be explicitly set, as this platform specification is necessary for now.Learnt from: cmraible Repo: TryGhost/Ghost PR: 23546 File: compose.yml:58-59 Timestamp: 2025-05-27T18:08:00.458Z Learning: The Ghost Docker Compose setup has two independent profiles: `ghost` profile (v0, runs all apps in a single container) and `split` profile (work in progress, runs Ghost server, admin, and frontend apps in separate services). The `split` profile will eventually replace `ghost` as the default.Learnt from: CR Repo: TryGhost/Ghost PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-11-25T14:28:50.351Z Learning: Use Docker Compose file composition for optional services: `yarn dev:analytics` for Tinybird, `yarn dev:storage` for MinIO, `yarn dev:all` for all optional servicesLearnt from: jonatansberg Repo: TryGhost/Ghost PR: 25552 File: e2e/helpers/environment/service-managers/dev-ghost-manager.ts:210-247 Timestamp: 2025-12-01T08:42:35.320Z Learning: In e2e/helpers/environment/service-managers/dev-ghost-manager.ts, the hardcoded volume name 'ghost-dev_shared-config' at line 231 is intentional. E2E tests run under the 'ghost-dev-e2e' project namespace but deliberately mount the shared-config volume from the main 'ghost-dev' project to access Tinybird credentials created by yarn dev:forward. This is cross-project volume sharing by design.
d8f8255 to
06adeb1
Compare
| ## Run `docker compose config --profiles` to see all available profiles | ||
| ## See https://docs.docker.com/compose/how-tos/profiles/ for more information | ||
| COMPOSE_PROFILES=ghost | ||
| # COMPOSE_PROFILES=stripe |
There was a problem hiding this comment.
Why remove the default profile? Just because it's not necessary?
There was a problem hiding this comment.
Yep! The default profile is part of the "legacy" docker environment, where by default only mysql + redis run.
9larsons
left a comment
There was a problem hiding this comment.
My only real concern here is mixing our configuration with .env and config.{x} and adding more confusion... however I think we'll never really avoid that since we need .env for Docker, unless we do some patching to pass through the config items.
Overall this looks good 🙌 . Definitely an improvement and step forward towards parity with our non-Docker dev setup.
no issue
Currently to run Ghost locally with Stripe with the new
yarn dev:forwardsetup, you need to install and run the Stripe CLI locally, and pass the webhook secret to Ghost via environment variable. This change makes it easier to run Ghost with Stripe with theyarn dev:forwardcommand.To use:
COMPOSE_PROFILES=stripeto.envfile at root of repoSTRIPE_SECRET_KEYto.envfile at root of repo (or export it in your shell)yarn dev:forwardHow it works
ghost-devstarts, the entrypoint reads WEBHOOK_SECRET from the shared volume into its environment before booting GhostNote
Enables local Stripe integration without a host-installed CLI by wiring a Dockerized Stripe CLI to Ghost via a shared config volume.
stripeservice (profilestripe) runningstripe/stripe-cliwith an entrypoint that fetchesSTRIPE_WEBHOOK_SECRET, writes it toshared-config/.env.stripe, and forwards webhooks to${GHOST_URL}/members/webhooks/stripe/ghost-deventrypoint to readSTRIPE_WEBHOOK_SECRETfrom shared config and export it asWEBHOOK_SECRETshared-configintoghost-devand adds optionaldepends_onforstripe; healthcheck waits for.env.stripecompose.yml, setsGHOST_URLfor thestripeservice;.env.examplehints enabling thestripeprofileWritten by Cursor Bugbot for commit 06adeb1. This will update automatically on new commits. Configure here.