Nix-based Docker builds; native, local, and multi-architecture development environments #25644
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a comprehensive Nix-based developer and Docker build environment for Ghost. Introduces a multi-stage Nix Docker build (.docker-nix/default.nix) with many per-app builders, a yarn offline cache, and a dockerImage derivation. Adds a top-level flake (flake.nix) exporting per-system packages, apps, and devShells (Linux-only docker exports conditional). Provides developer tooling and orchestration (.nix/dev-shell.nix, .nix/process-compose.yaml), helper scripts and utilities (.nix/precache-package, .nix/update-yarn-hash), git hook (.nix/githooks/pre-commit), treefmt config, minimal /etc files, direnv support (.envrc), and a CI workflow to build and publish multi-arch images. Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
onethirtyfive
force-pushed
the
feat/nix-docker-builds
branch
from
775ea53 to
5d4313e
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: 4
🧹 Nitpick comments (4)
.nix/README.md (1)
77-86: Add language specifier to code block.The fenced code block on line 77 should specify a language for proper syntax highlighting.
Apply this diff:
## Files -``` +```plaintext flake.nix # Package definitions.nix/process-compose.yaml (1)
3-119: Good overall orchestration; a couple of robustness checks worth consideringThe process layout and dependency chain (MySQL → mysql-init → Ghost, plus Redis/Mailpit health) look well thought through. A few points to consider tightening:
Unquoted
$PWDin commands and probes
Paths like--datadir=$PWD/.dev-data/mysql,--socket=$PWD/.dev-data/mysql.sock, and--dir $PWD/.dev-data/rediswill break if the checkout path contains spaces or shell metacharacters. Using"${PWD}/.dev-data/mysql"-style quoting inside the command blocks would make this more robust across environments.Environment interpolation semantics for
GHOST_DEV_APP_FLAGS
The line
- GHOST_DEV_APP_FLAGS=${GHOST_DEV_APP_FLAGS:-}
assumes docker‑compose–style interpolation with defaulting. Please double‑check thatprocess-composehonors this syntax the same way; if not, you may end up passing the literal string${GHOST_DEV_APP_FLAGS:-}instead of propagating the host variable or an empty string. If interpolation isn’t supported, consider either:
- GHOST_DEV_APP_FLAGS(let the tool pass through the host value), or- Using a wrapper script (
ghost-dev) that exports this variable before runningyarn dev.MySQL/knex-migrator assumptions
mysql-initconnects to theghostdatabase over the socket. That matches the later Ghost config, but it assumes knex-migrator (and its config) will create the database on first run. If any recent Ghost changes altered that behavior, this step might fail repeatedly; worth confirming once against a clean.dev-datadirectory.None of these are blockers, but addressing (1) and confirming (2)/(3) would make the dev experience more resilient.
.github/workflows/ci-docker-nix.yml (1)
63-75: Address ShellCheck warnings for quoting and unused variables in workflow scriptsThe static analysis hints for this workflow are mostly ShellCheck warnings that are easy to clean up and will make the scripts more robust:
Quote output files in
echo >>redirections
In both the cache-check and summary steps you have patterns like:echo "cache_hit=true" >> $GITHUB_OUTPUT ... echo "## ..." >> $GITHUB_STEP_SUMMARYPrefer quoting the target:
echo "cache_hit=true" >> "$GITHUB_OUTPUT" echo "## ..." >> "$GITHUB_STEP_SUMMARY"This fixes SC2086 and is generally safer.
Quoting in loops and skopeo invocations
For example:TAGS="${{ steps.meta.outputs.tags }}" for tag in $TAGS; do ... nix run nixpkgs#skopeo -- copy \ --preserve-digests \ docker-archive:result \ docker://$tag-${{ matrix.arch }} doneThe
for tag in $TAGSis intentionally unquoted to split on newlines, which is fine, but quoting the composed reference improves safety:docker://"$tag"-${{ matrix.arch }}Similarly, quoting other expanded variables in skopeo/stat/bc calls (as per the SC2086 hints) is a low-risk hardening step.
Remove or use
IMAGE_INFOin the inspect stepIMAGE_INFO=$(nix run nixpkgs#skopeo -- inspect docker-archive:result)Since
IMAGE_INFOis never used, either drop the assignment (just run the command) or parse and surface something from it (e.g. digest, created time). That will address SC2034 and make the step’s intent clearer.These are all minor improvements, but fixing them will keep actionlint/ShellCheck green and reduce the chance of subtle shell issues in the future.
Also applies to: 90-145, 164-201
.docker-nix/default.nix (1)
410-416: Consider removing non-existent paths from PATH.
/usr/bin:/bindon't exist in Nix-based containers since all binaries are in the Nix store. While this won't cause errors, it adds noise to PATH lookups.Env = [ "NODE_ENV=development" "NX_DAEMON=true" "GHOST_DEV_IS_DOCKER=true" "LD_LIBRARY_PATH=${commonEnv.LD_LIBRARY_PATH}" - "PATH=/home/ghost/node_modules/.bin:/usr/bin:/bin:${nodejs}/bin:${pkgs.yarn}/bin:${pkgs.stripe-cli}/bin" + "PATH=/home/ghost/node_modules/.bin:${nodejs}/bin:${pkgs.yarn}/bin:${pkgs.stripe-cli}/bin" ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.docker-nix/default.nix(1 hunks).docker-nix/etc/group(1 hunks).docker-nix/etc/nsswitch.conf(1 hunks).docker-nix/etc/passwd(1 hunks).envrc(1 hunks).github/workflows/ci-docker-nix.yml(1 hunks).gitignore(1 hunks).nix/README.md(1 hunks).nix/dev-shell.nix(1 hunks).nix/githooks/README.md(1 hunks).nix/githooks/pre-commit(1 hunks).nix/precache-package/default.nix(1 hunks).nix/precache-package/precache-package.sh(1 hunks).nix/process-compose.yaml(1 hunks).nix/treefmt.nix(1 hunks).nix/update-yarn-hash/default.nix(1 hunks).nix/update-yarn-hash/update-yarn-hash.sh(1 hunks)flake.nix(1 hunks)
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
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.
📚 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:
.nix/dev-shell.nix.nix/process-compose.yaml.gitignore.docker-nix/default.nix
📚 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:
.nix/dev-shell.nix.nix/process-compose.yaml.docker-nix/default.nix
📚 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:
.nix/dev-shell.nix.nix/process-compose.yaml.docker-nix/default.nix
📚 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:
.nix/process-compose.yaml.docker-nix/default.nix
📚 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:
.nix/process-compose.yaml.docker-nix/default.nix
📚 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:
.nix/process-compose.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:
.nix/process-compose.yaml.docker-nix/default.nix
📚 Learning: 2025-06-19T22:57:05.880Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23941
File: .github/workflows/ci.yml:911-914
Timestamp: 2025-06-19T22:57:05.880Z
Learning: In Ghost, when NODE_ENV is set to testing-mysql, Ghost uses config.testing-mysql.json configuration which sets the server port to 2369 instead of the default 2368. This also applies to other testing environments like testing and testing-browser.
Applied to files:
.nix/process-compose.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:
.nix/process-compose.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: Always use `yarn` (v1) for all commands in this Yarn v1 + Nx monorepo
Applied to files:
.nix/update-yarn-hash/default.nix
📚 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: Applies to e2e/**/*.{ts,js} : E2E tests should use Playwright with Docker container isolation
Applied to files:
.gitignore
📚 Learning: 2025-11-24T17:28:51.262Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/comments-ui/.cursor/rules/playwright-e2e.mdc:0-0
Timestamp: 2025-11-24T17:28:51.262Z
Learning: Applies to apps/comments-ui/{package.json,**/.github/workflows/**,**/playwright.config.{ts,js},**/*.{sh,bash}} : Set PLAYWRIGHT_REPORTER=list environment variable when running Playwright e2e tests as an AI agent for better parsing
Applied to files:
.gitignore
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
.gitignore
📚 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: Check `e2e/CLAUDE.md` for detailed E2E testing guidance and debugging tips
Applied to files:
.gitignore
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Always follow ADRs in `../adr/` folder (ADR-0001: AAA pattern, ADR-0002: Page Objects)
Applied to files:
.gitignore
📚 Learning: 2025-11-26T11:05:59.314Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-11-26T11:05:59.314Z
Learning: Applies to apps/shade/{src,test}/**/*.{ts,tsx,js} : Run `yarn lint` after making changes to fix any ESLint errors and warnings before committing
Applied to files:
.nix/githooks/pre-commit
📚 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: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
Applied to files:
.docker-nix/default.nix
📚 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:build` to build Docker images and delete ephemeral volumes
Applied to files:
.docker-nix/default.nix
🪛 actionlint (1.7.9)
.github/workflows/ci-docker-nix.yml
46-46: shellcheck reported issue in this script: SC2086:info:13:29: Double quote to prevent globbing and word splitting
(shellcheck)
46-46: shellcheck reported issue in this script: SC2086:info:9:28: Double quote to prevent globbing and word splitting
(shellcheck)
110-110: shellcheck reported issue in this script: SC2086:info:13:14: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2034:warning:4:1: IMAGE_INFO appears unused. Verify use (or export if used externally)
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:10:69: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:11:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:12:37: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:13:48: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:14:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:15:49: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:16:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:17:60: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2129:style:10:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:10:83: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:1:50: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:3:63: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:4:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:7:24: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:9:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
🪛 LanguageTool
.nix/README.md
[uncategorized] ~94-~94: The official name of this software platform is spelled with a capital “H”.
Context: ...cache-package/precache-package.sh, and .github/workflows/ci-docker-nix.yml`. Cachix is...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
.nix/README.md
23-23: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (32)
.docker-nix/etc/group (1)
1-2: LGTM!The group file format is correct and defines appropriate system groups for the containerized Ghost environment.
.nix/githooks/README.md (1)
1-28: LGTM!The documentation clearly explains the rationale, installation options, and workflow for the git hooks. Well-structured and informative.
.nix/README.md (1)
1-108: Excellent documentation with clear production transition notes.The README thoroughly explains the Nix infrastructure, benefits, and trade-offs. The explicit call-out about replacing the personal Cachix cache (
hello-stocha) for production is important and well-documented..nix/treefmt.nix (1)
1-13: LGTM!Clean formatting configuration using the modern nixfmt-rfc-style. The includes pattern appropriately covers all Nix files in the repository.
.nix/update-yarn-hash/update-yarn-hash.sh (1)
1-112: Well-structured script with good error handling.The script has excellent error handling, rollback logic on failure, and clear user feedback throughout the hash update process. The trap for cleanup and verification step are particularly well done.
.nix/update-yarn-hash/default.nix (1)
1-13: Nix app wrapper forupdate-yarn-hashlooks solidThe
writeShellApplicationwrapper is minimal and correct for exposingupdate-yarn-hash.shvianix run .#update-yarn-hash, with appropriate runtime inputs fornix,sed, andgrep. Just keep in mind that ifupdate-yarn-hash.shgrows additional tool dependencies (e.g.git), they should be added here to keep the app hermetic..nix/dev-shell.nix (2)
60-148: Dev shell composition and native-build flags look appropriateThe chosen packages (Node 22, Yarn, mysql80, redis, mailpit, vips, toolchain, process-compose, Cachix) and the shellHook exports for
PKG_CONFIG_PATH,LD_LIBRARY_PATH,npm_config_sharp_*,npm_config_build_from_source, andPYTHON/GYP_PYTHONare consistent with how Ghost’s native dependencies (sharp, sqlite3, node-gyp) typically need to be wired under Nix. Disabling Husky in the dev shell is also a sensible choice for this workflow.No changes needed here from a correctness perspective.
21-57:ghost-dev-setupmay clobber a tracked Casper theme directoryThe setup script aggressively replaces the
casperdirectory:if [ ! -d "ghost/core/content/themes/casper/.git" ]; then echo "📦 Cloning Casper theme..." if [ -d "ghost/core/content/themes/casper" ]; then rm -rf ghost/core/content/themes/casper fi git clone --depth 1 https://github.com/TryGhost/Casper.git ghost/core/content/themes/casper fiIf
ghost/core/content/themes/casperis tracked in this repo or managed outside a git clone, runningghost-dev-setupwill delete tracked files and replace the directory with a fresh clone, leaving contributors with uncommitted diffs and potential version mismatches.Consider tightening the logic:
- Check for directory absence (
[ ! -d "ghost/core/content/themes/casper" ]) instead of.gitabsence, or- Gate the destructive behavior behind an explicit flag/env var for opt-in behavior.
This prevents unexpected workspace mutations when developers run the setup helper.
.docker-nix/etc/nsswitch.conf (1)
1-3: Minimalnsswitch.confis appropriate for the container use caseUsing local files for
passwd/groupandfiles dnsforhostsis a sensible minimal configuration for this image; nothing stands out as problematic..docker-nix/etc/passwd (1)
1-2: Container user definitions look consistent; consider shell choice if hardening laterDefining
rootand an unprivilegedghostuser with UID/GID 1000 matches typical Ghost container setups and should integrate cleanly with the rest of the Nix-based image.If you later decide to harden interactive access, you might switch the login shells to a non-interactive shell (e.g.
/usr/sbin/nologin) for production images while keeping/bin/bashfor a dedicated debug/dev variant..nix/precache-package/default.nix (1)
1-12:precache-packageNix app wiring looks correctThe wrapper cleanly exposes
precache-package.shasnix run .#precache-package, and includes the right tools (nix,cachix) inruntimeInputsfor a hermetic execution environment. This aligns well with the flake/app model you’re using elsewhere..nix/precache-package/precache-package.sh (1)
1-136: Precache script is well-structured and robustThis script is nicely put together:
set -euo pipefailplus careful use of arrays (ARGS,NIX_EXTRA_ARGS) avoids common Bash pitfalls.- The
--allow-uncommittedguard for path-based flake URLs is a good safety net to keep CI caches aligned with committed state.- Cache probing via
nix eval ...outPath+nix path-infoagainst the Cachix store is efficient, and falling back tocachix watch-exec "$CACHE_NAME" -- nix build ...is the right way to prime the cache.Under
set -e, the explicitif [ -z "$BUILD_RESULT" ]check is mostly defensive (a failingnix buildwill already abort the script), but it doesn’t hurt. Overall this is solid..github/workflows/ci-docker-nix.yml (1)
44-88: Verify fork PR flake URL behavior and matrix job output passingTwo structural concerns require verification against the actual workflow:
Fork PR compatibility of the flake URL
In both the cache-check and build steps, the workflow uses:SHA="${{ github.event.pull_request.head.sha || github.sha }}" nix eval/build "github:${{ github.repository }}/$SHA#packages.${{ matrix.system }}.dockerImage..."For PRs from forks,
github.event.pull_request.head.shapoints to a commit in the fork, whilegithub.repositoryis the base repo. The flake URLgithub:${{ github.repository }}/$SHAmay not resolve if that commit doesn't exist in the base repository.Confirm this works as intended for forked PRs. If issues arise, consider using
github:${{ github.event.pull_request.head.repo.full_name }}/$SHAfor fork PRs, or switching to path-based flakes (.#packages.${{ matrix.system }}.dockerImage) during pull_request events.Job outputs from matrix jobs
The workflow defines outputs in thebuildjob and passes them to downstream jobs vianeeds.build.outputs. GitHub Actions matrix job output passing behavior can be inconsistent—verify thatneeds.build.outputs.image-tagsresolves correctly and contains the intended values, not an empty string or arbitrary matrix leg output.Test the workflow end-to-end to ensure both scenarios (fork and base repo PRs, branch pushes) execute without errors and produce correct outputs.
.docker-nix/default.nix (11)
1-12: LGTM!Clear documentation header explaining the key differences from the traditional Dockerfile approach. The function signature is well-structured with appropriate inputs.
27-41: LGTM!The commonEnv configuration correctly sets up native module compilation from source against Nix-provided libraries. Setting
npm_config_sqlite3_binary_host = ""effectively disables prebuilt binary downloads, and the LD_LIBRARY_PATH includes all necessary shared libraries.
83-86: Hash maintenance required when yarn.lock changes.The hardcoded hash will need updating whenever
yarn.lockis modified. Verify theupdate-yarn-hashapp in the flake is documented for maintainers.
105-121: LGTM!The build phase correctly handles:
- Disabling Husky to avoid git hook issues in sandbox
- Frozen lockfile + offline mode for reproducibility
- Shebang patching for Nix store paths
- Native module rebuilding against Nix libraries
- Cleanup of
.o,.a, andobj.targetartifacts to reduce image size
139-177: LGTM!The
mkWorkspaceBuildhelper is well-designed:
- Abstracts common workspace build patterns
- The
extraDepsmechanism correctly propagates dependent build artifacts with proper write permissions- Conditional directory copies handle varying output structures (
dist,es,types,umd,public)
179-251: LGTM!The workspace builders correctly model the dependency graph:
shadeandadmin-x-design-systemare leaf dependenciesadmin-x-frameworkdepends on bothcommonAdminDepsabstracts the shared dependency set- Individual admin-x apps correctly inherit these dependencies
253-278: LGTM!The ghost-core-tsc-builder correctly builds TypeScript and selectively copies the generated JS files from
core/server. The find/while loop pattern works for this use case where filenames don't contain special characters.
303-333: LGTM!The
admin-ember-buildercorrectly:
- Assembles dependent app builds (stats, posts, admin-x-settings, activitypub) before the Ember build
- Creates the target directory structure with
mkdir -p- Outputs both the admin dist and built artifacts separately for later assembly
335-363: LGTM!The
ghost-appderivation correctly assembles all component outputs into the final application structure. Since it only performs file copy operations, the implicit stdenv tools are sufficient.
423-433: LGTM!The
extraCommandscorrectly sets up:
- The working directory with writable permissions for development
- A world-writable
/tmpwith sticky bit (1777)maxLayers = 100enables efficient layer caching and reuse
365-382: LGTM!Exporting individual builders alongside the final
dockerImageenables faster iteration during development - developers can rebuild specific components without rebuilding the entire image.flake.nix (8)
10-15: Verify the Cachix substituter is appropriate for upstream use.The
hello-stocha.cachix.orgsubstituter appears to be a personal/project-specific cache. For a mainline Ghost repository:
- Ensure this cache is maintained and trusted for all contributors
- Consider using an official Ghost organization cache for long-term maintenance
25-30: LGTM!Excluding
x86_64-darwinis a reasonable decision given the declining Intel Mac usage. The comment documents the intentional omission.
47-52: LGTM!Standard treefmt-nix integration providing per-system formatters and CI-compatible formatting checks via
nix flake check.
54-72: LGTM!The apps are cleanly structured following flake conventions.
update-yarn-hashis essential for maintaining the yarn offline cache hash in.docker-nix/default.nix.
79-111: LGTM!The source filtering is comprehensive and well-documented:
- Excludes infrastructure files (flake.nix, compose.yml, etc.)
- Excludes CI/IDE/AI assistant directories
- Excludes documentation
- Correctly includes
.docker-nix/(not in exclusion list) since it's needed- Composes with
cleanSourceFilterfor standard exclusions (.git, caches)
113-147: LGTM!The conditional Docker output handling is correct:
dockerBuildsis only imported on Linux wherebuildLayeredImageis available- Non-Linux platforms receive an empty attribute set, preventing evaluation errors
- Individual builders are exposed alongside
dockerImagefor development iteration
127-133: LGTM!The
devpackage provides a convenient entry point usingprocess-composefor local service orchestration, referencing the config from.nix/process-compose.yaml.
150-152: LGTM!Standard devShell setup delegating to a separate module for better organization.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.gitignore (1)
206-208: Remove.envrcfrom.gitignore— it conflicts with the tracked file.The PR adds
.envrcas a tracked file, but line 207 ignores it. Since.envrc.local(line 211) is already ignored for local customizations, the base.envrcshould remain tracked.# direnv environment loader files -.envrc +# .envrc is tracked; use .envrc.local for local customizations.envrc (1)
2-4: Consider updating nix-direnv to 3.0.7.Version 3.0.7 was released in May 2025. Update the version reference and SHA256 hash from the official release.
🧹 Nitpick comments (10)
.nix/README.md (3)
21-27: Minor: Add heading markup and clarify data source.The emphasis on line 23 should be a heading per markdown conventions. Also, consider linking to specific CI run(s) for the performance claims.
## Measured Results -**Build time with warm cache: 15-20 min → 3-6 min (70% faster)** +### Build time with warm cache: 15-20 min → 3-6 min (70% faster) On self-hosted runners with a persistent Nix store, this could get as low as 1-3 minutes. -Data from GitHub Actions runs. +Data from GitHub Actions runs. <!-- Consider adding link to specific run -->
77-86: Minor: Add language specifier to code block.Adding a language identifier (e.g.,
textorplaintext) satisfies markdown linters and improves rendering consistency.-``` +```text flake.nix # Package definitions
92-95: Capitalize "GitHub" and clarify cache transition path.Minor capitalization fix. The transparency about the personal cache is appreciated — good callout for production adoption.
-Cachix is a trusted provider of Nix binary caching as a service—you're also free to self-host your own, which can be as simple as an S3 bucket. +Cachix is a trusted provider of Nix binary caching as a service — you're also free to self-host your own, which can be as simple as an S3 bucket..nix/process-compose.yaml (2)
64-66: Unusual Redis readiness probe command.The command
redis-cli --raw incr pingwill increment a key named "ping" rather than checking server health. The standard Redis ping command isredis-cli ping, which returns "PONG" if the server is healthy.readiness_probe: exec: - command: "redis-cli --raw incr ping" + command: "redis-cli ping"
25-28: Consider documenting memory requirements or making buffer sizes configurable.The InnoDB settings allocate ~1.5GB for buffers (
innodb-buffer-pool-size=1G+innodb-log-buffer-size=500M). This is fine for development machines with sufficient RAM but could cause issues on constrained systems. Consider adding a comment about memory requirements or parameterizing via environment variables..nix/dev-shell.nix (1)
33-42: Consider warning before removing modified Casper theme.The script removes the existing Casper directory without checking for local modifications. A developer with uncommitted changes to the theme could lose work.
if [ ! -d "ghost/core/content/themes/casper/.git" ]; then echo "📦 Cloning Casper theme..." if [ -d "ghost/core/content/themes/casper" ]; then + echo "⚠️ Removing existing (non-git) casper directory..." rm -rf ghost/core/content/themes/casper fi git clone --depth 1 https://github.com/TryGhost/Casper.git ghost/core/content/themes/casperAlternatively, consider using
git status --porcelainto detect uncommitted changes in the existing directory before removal..github/workflows/ci-docker-nix.yml (4)
44-61: Quote shell variables to prevent word splitting.Per static analysis, the
$OUT_PATHvariable should be double-quoted to prevent word splitting and globbing issues.# Check if that exact store path exists in Cachix - if [ -n "$OUT_PATH" ] && nix path-info --store "https://${{ env.CACHIX_CACHE }}.cachix.org" \ - "$OUT_PATH" >/dev/null 2>&1; then + if [ -n "$OUT_PATH" ] && nix path-info --store "https://${{ env.CACHIX_CACHE }}.cachix.org" "$OUT_PATH" >/dev/null 2>&1; then
116-124: Quote the$TAGSvariable in the loop.The
$TAGSvariable should be quoted to properly handle any tags containing special characters.# Push with all tags, appending arch suffix - TAGS="${{ steps.meta.outputs.tags }}" - for tag in $TAGS; do + for tag in ${{ steps.meta.outputs.tags }}; do echo "Pushing: $tag-${{ matrix.arch }}" nix run nixpkgs#skopeo -- copy \ --preserve-digests \ docker-archive:result \ docker://$tag-${{ matrix.arch }} doneNote: Since the tags come directly from
docker/metadata-actionoutput, you can iterate directly over the expression. Alternatively, use an array:readarray -t TAGS <<< "${{ steps.meta.outputs.tags }}"thenfor tag in "${TAGS[@]}".
126-145: Remove unusedIMAGE_INFOvariable.The
IMAGE_INFOvariable is assigned but never used, as flagged by static analysis.- name: Inspect image run: | IMAGE_TAG=$(echo "${{ steps.meta.outputs.tags }}" | head -n1)-${{ matrix.arch }} - # Inspect tarball with skopeo (no need to load into Docker) - IMAGE_INFO=$(nix run nixpkgs#skopeo -- inspect docker-archive:result) - # Get image size from the tarball file IMAGE_SIZE_BYTES=$(stat -c%s result 2>/dev/null || stat -f%z result 2>/dev/null) IMAGE_SIZE_GB=$(echo "scale=2; $IMAGE_SIZE_BYTES / 1024 / 1024 / 1024" | bc) - echo "## Docker Image Analysis (Nix Build - ${{ matrix.arch }})" >> $GITHUB_STEP_SUMMARY + { + echo "## Docker Image Analysis (Nix Build - ${{ matrix.arch }})" + echo "" + echo "**Image:** \`$IMAGE_TAG\`" + echo "**Architecture:** ${{ matrix.arch }}" + echo "" + echo "**Tarball Size:** ${IMAGE_SIZE_GB} GB" + echo "" + echo "**Built with:** Nix flakes + Cachix binary cache" + } >> "$GITHUB_STEP_SUMMARY"
164-187: Apply consistent variable quoting in manifest creation.Same quoting considerations apply here. Also, the manifest commands should handle potential failures gracefully.
- name: Create and push multi-arch manifests run: | - # Get the base tags (without arch suffix) - TAGS="${{ needs.build.outputs.image-tags }}" - - for tag in $TAGS; do + for tag in ${{ needs.build.outputs.image-tags }}; do echo "Creating manifest for: $tag" # Create manifest pointing to both arch-specific images docker manifest create "$tag" \ "$tag-x86_64" \ "$tag-aarch64"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.docker-nix/default.nix(1 hunks).docker-nix/etc/group(1 hunks).docker-nix/etc/nsswitch.conf(1 hunks).docker-nix/etc/passwd(1 hunks).envrc(1 hunks).github/workflows/ci-docker-nix.yml(1 hunks).gitignore(1 hunks).nix/README.md(1 hunks).nix/dev-shell.nix(1 hunks).nix/githooks/README.md(1 hunks).nix/githooks/pre-commit(1 hunks).nix/precache-package/default.nix(1 hunks).nix/precache-package/precache-package.sh(1 hunks).nix/process-compose.yaml(1 hunks).nix/treefmt.nix(1 hunks).nix/update-yarn-hash/default.nix(1 hunks).nix/update-yarn-hash/update-yarn-hash.sh(1 hunks)flake.nix(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- .nix/update-yarn-hash/update-yarn-hash.sh
- .nix/githooks/pre-commit
- .nix/treefmt.nix
- .docker-nix/etc/group
- .docker-nix/etc/passwd
- .docker-nix/etc/nsswitch.conf
- .nix/precache-package/precache-package.sh
- .nix/githooks/README.md
🧰 Additional context used
🧠 Learnings (18)
📓 Common learnings
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: niranjan-uma-shankar
Repo: TryGhost/Ghost PR: 24557
File: apps/admin-x-settings/src/components/settings/general/TimeZone.tsx:7-7
Timestamp: 2025-08-01T12:44:07.467Z
Learning: In Ghost development, PRs may depend on unpublished changes from SDK packages. When this occurs, TypeScript compilation errors for missing exports are expected and documented in the PR description until the dependency packages are published and updated. This is normal workflow for cross-repository feature development.
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: Run `yarn docker:dev` to start Ghost in Docker with hot reload
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: 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:
.docker-nix/default.nix.nix/process-compose.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:
.docker-nix/default.nix.nix/dev-shell.nix.nix/process-compose.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:
.docker-nix/default.nix.nix/dev-shell.nix.nix/process-compose.yaml
📚 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:
.docker-nix/default.nix.gitignore.nix/process-compose.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:
.docker-nix/default.nix.nix/process-compose.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: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
Applied to files:
.docker-nix/default.nix
📚 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:build` to build Docker images and delete ephemeral volumes
Applied to files:
.docker-nix/default.nix
📚 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:
.docker-nix/default.nix.nix/process-compose.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: Applies to e2e/**/*.{ts,js} : E2E tests should use Playwright with Docker container isolation
Applied to files:
.gitignore
📚 Learning: 2025-11-24T17:28:51.262Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/comments-ui/.cursor/rules/playwright-e2e.mdc:0-0
Timestamp: 2025-11-24T17:28:51.262Z
Learning: Applies to apps/comments-ui/{package.json,**/.github/workflows/**,**/playwright.config.{ts,js},**/*.{sh,bash}} : Set PLAYWRIGHT_REPORTER=list environment variable when running Playwright e2e tests as an AI agent for better parsing
Applied to files:
.gitignore
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Use Playwright's auto-waiting capabilities and run tests multiple times to ensure stability
Applied to files:
.gitignore
📚 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: Check `e2e/CLAUDE.md` for detailed E2E testing guidance and debugging tips
Applied to files:
.gitignore
📚 Learning: 2025-11-24T17:29:43.865Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: e2e/AGENTS.md:0-0
Timestamp: 2025-11-24T17:29:43.865Z
Learning: Applies to e2e/**/*.test.ts : Always follow ADRs in `../adr/` folder (ADR-0001: AAA pattern, ADR-0002: Page Objects)
Applied to files:
.gitignore
📚 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: Always use `yarn` (v1) for all commands in this Yarn v1 + Nx monorepo
Applied to files:
.nix/update-yarn-hash/default.nix
📚 Learning: 2025-06-19T22:57:05.880Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23941
File: .github/workflows/ci.yml:911-914
Timestamp: 2025-06-19T22:57:05.880Z
Learning: In Ghost, when NODE_ENV is set to testing-mysql, Ghost uses config.testing-mysql.json configuration which sets the server port to 2369 instead of the default 2368. This also applies to other testing environments like testing and testing-browser.
Applied to files:
.nix/process-compose.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:
.nix/process-compose.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:
.nix/process-compose.yaml
🪛 actionlint (1.7.9)
.github/workflows/ci-docker-nix.yml
46-46: shellcheck reported issue in this script: SC2086:info:13:29: Double quote to prevent globbing and word splitting
(shellcheck)
46-46: shellcheck reported issue in this script: SC2086:info:9:28: Double quote to prevent globbing and word splitting
(shellcheck)
110-110: shellcheck reported issue in this script: SC2086:info:13:14: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2034:warning:4:1: IMAGE_INFO appears unused. Verify use (or export if used externally)
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:10:69: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:11:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:12:37: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:13:48: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:14:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:15:49: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:16:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:17:60: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2129:style:10:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:10:83: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:1:50: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:3:63: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:4:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:7:24: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:9:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
🪛 LanguageTool
.nix/README.md
[uncategorized] ~94-~94: The official name of this software platform is spelled with a capital “H”.
Context: ...cache-package/precache-package.sh, and .github/workflows/ci-docker-nix.yml`. Cachix is...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
.nix/README.md
23-23: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (17)
.gitignore (1)
209-215: Nix-related ignore entries look good.The additions for
.direnv/,.envrc.local,result,result-*,.claude/, and.dev-data/are appropriate for a Nix development environment — these are all local/transient artifacts that shouldn't be tracked..envrc (1)
6-10: Clean direnv setup with good extensibility.The pattern of sourcing
.envrc.localfor local overrides is a good practice. The--accept-flake-configflag is necessary for the Cachix substituters defined inflake.nix..docker-nix/default.nix (5)
83-86: Yarn offline cache hash management is well-designed.The hardcoded hash requires updates when
yarn.lockchanges, but this is addressed by theupdate-yarn-hashscript and pre-commit hook mentioned in the README. Good balance between reproducibility and maintainability.
105-121: Native module rebuild approach is correct for Nix.Rebuilding
sqlite3,sharp, andre2against Nix-provided libraries ensures proper sandboxed builds. The cleanup of.o,.a, andobj.targetartifacts is a good optimization for image size.
166-176: Conditional output copying handles varying workspace structures.The
[ -d ... ] && cppattern gracefully handles workspaces that produce different output directories. This flexibility is appropriate given the variety of build outputs across Ghost workspaces.
384-433: Docker image configuration looks solid.The
buildLayeredImagesetup includes appropriate runtime dependencies, working directory, exposed port, and environment variables. ThemaxLayers = 100provides good layer granularity for caching. TheextraCommandscorrectly sets up/tmpwith sticky bit permissions.
410-416: Verify if Ghost codebase depends on /usr/bin or /bin paths, or remove them.The PATH includes
/usr/bin:/bin, but pure Nix environments do not populate these directories by default—Nix keeps binaries in the store and exposes them via explicit PATH entries instead. If Ghost scripts or dependencies rely on finding binaries in these conventional locations, add symlinks during container setup or add the required packages to the Nix environment explicitly. Otherwise, remove these paths since they are unlikely to be available and may cause false expectations about where tools can be found..nix/precache-package/default.nix (1)
1-11: Clean shell application pattern.Using
writeShellApplicationwithruntimeInputsis the idiomatic Nix approach — it ensuresnixandcachixare available and applies strict shell mode automatically. Separating the script content into a.shfile aids readability and shellcheck linting..nix/update-yarn-hash/default.nix (1)
1-13: LGTM!Clean use of
pkgs.writeShellApplicationwhich automatically enables bash strict mode and validates the script with shellcheck. The runtime inputs are appropriately specified..nix/process-compose.yaml (1)
95-119: LGTM!Service dependencies are properly ordered (mysql-init must complete before ghost starts), environment variables are correctly wired, and the restart policy is reasonable for development.
.nix/dev-shell.nix (2)
60-98: LGTM!Comprehensive package selection for Ghost development. The inclusion of image processing libraries for
sharp, Python with setuptools for node-gyp, and process-compose for orchestration covers the development requirements well.
100-148: Well-structured shellHook with helpful onboarding.The environment configuration correctly sets up paths for native module compilation, and the Cachix onboarding message guides developers to enable binary caching. The multiple Python environment variables (
PYTHON,npm_config_python,GYP_PYTHON) ensure compatibility across different node-gyp invocation methods..github/workflows/ci-docker-nix.yml (1)
16-42: Well-designed workflow with smart cache optimization.The cache-check step that evaluates the output path before building is a clever optimization. Combined with conditional disk cleanup, this should significantly reduce CI time when the Cachix cache is warm. The matrix strategy with
fail-fast: falseensures both architectures complete independently.flake.nix (4)
1-16: LGTM!Clean flake structure with proper input declarations. The
nixConfigblock correctly configures the Cachix binary cache substituter for faster builds.
79-111: Source filter logic is comprehensive.The exclusion list correctly filters out Nix infrastructure, CI, documentation, and IDE-specific directories while preserving
.docker-nix/which is needed for Docker builds. The combination withcleanSourceFilterhandles.gitand build artifacts.One minor observation: the filter could be slightly more maintainable as a list, but the current inline approach is clear enough.
113-148: Correct platform-conditional Docker package exports.The
if pkgs.stdenv.isLinuxguards correctly prevent attempting to build Docker images on Darwin systems where the Linux-specific derivations aren't supported. The merged attribute set pattern is idiomatic Nix.
150-153: LGTM!Simple and effective devShell export that enables
nix developfor all supported systems.
onethirtyfive
force-pushed
the
feat/nix-docker-builds
branch
from
5d4313e to
2145b29
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (10)
.nix/README.md (2)
23-23: Convert bold text to a section heading.Line 23 uses bold emphasis (
**...**) where a markdown heading would be more appropriate per markdownlint rules.Apply this diff:
-**Build time with warm cache: 15-20 min → 3-6 min (70% faster)** +## Build time with warm cache: 15-20 min → 3-6 min (70% faster)
77-86: Add language identifier to code block.The code block starting at line 77 lists file paths without a language specifier. Add
textor leave unspecified per markdown best practices.Apply this diff:
-``` +```text flake.nix # Package definitions .docker-nix/default.nix # Docker image build .nix/ ├── dev-shell.nix # Development environment ├── update-yarn-hash/ # Hash update automation ├── precache-package/ # CI cache warming ├── githooks/ # Pre-commit validation └── process-compose.yaml # Local service orchestration</blockquote></details> <details> <summary>.nix/process-compose.yaml (3)</summary><blockquote> `41-55`: **Consider explicitly creating the `ghost` MySQL database before running migrations** `mysql-init` assumes a `ghost` database exists, but unlike the Docker-compose flow there’s no `MYSQL_DATABASE` bootstrap here. If `knex-migrator init` doesn’t create the DB itself, this step will fail and `ghost` will never start because it depends on `mysql-init` completing successfully. A minimal hardening would be to create the database up front: ```diff mysql-init: command: | - cd ghost/core - yarn knex-migrator init + # Ensure database exists + mysql -u root -S .dev-data/mysql.sock -e "CREATE DATABASE IF NOT EXISTS ghost;" + + cd ghost/core + yarn knex-migrator initThis keeps the initializer idempotent and makes the stack more robust if the data directory is wiped or partially initialized.
3-7: Double‑check env interpolation semantics for${PWD}and${GHOST_DEV_APP_FLAGS:-}This config relies on process-compose expanding
${PWD}and${GHOST_DEV_APP_FLAGS:-}in environment entries. Depending on process-compose’s interpolation rules, these may end up as literal strings rather than substituted values.If interpolation doesn’t work as expected, consider:
- Passing
PWDandGHOST_DEV_APP_FLAGSvia the shell wrapper (e.g.ghost-dev/nix run) instead of embedding${...}in YAML.- Or sticking to strictly supported
${VAR}/${VAR:-default}semantics confirmed in process-compose docs.That will avoid surprises where Ghost sees a literal
${PWD}/...socket path or${GHOST_DEV_APP_FLAGS:-}as the flag value.Also applies to: 104-116
20-28: Optional: reduce MySQL InnoDB buffer sizes for lighter dev environments
innodb-buffer-pool-size=1Gandinnodb-log-buffer-size=500Mare quite large for a local dev stack and may cause pressure on laptops or CI runners.If you don’t need that much headroom for development, you could drop these to more modest values (e.g. 256M/64M) to lower the memory footprint without impacting typical dev workloads.
.github/workflows/ci-docker-nix.yml (2)
44-61: Tighten shell quoting and drop unusedIMAGE_INFOto satisfy shellcheck/actionlintThe scripts are generally solid, but a few tweaks will remove actionlint noise and harden against word-splitting:
- Quote file/output variables (
$GITHUB_OUTPUT,$GITHUB_STEP_SUMMARY) on redirection.- Iterate over tags safely and quote them when used in URLs.
- Remove the unused
IMAGE_INFOassignment.For example:
- echo "cache_hit=true" >> $GITHUB_OUTPUT + echo "cache_hit=true" >> "$GITHUB_OUTPUT" ... - TAGS="${{ steps.meta.outputs.tags }}" - for tag in $TAGS; do - echo "Pushing: $tag-${{ matrix.arch }}" + TAGS="${{ steps.meta.outputs.tags }}" + for tag in $TAGS; do + echo "Pushing: ${tag}-${{ matrix.arch }}" nix run nixpkgs#skopeo -- copy \ --preserve-digests \ docker-archive:result \ - docker://$tag-${{ matrix.arch }} + "docker://${tag}-${{ matrix.arch }}" done ... - IMAGE_INFO=$(nix run nixpkgs#skopeo -- inspect docker-archive:result) + # Optional: inspect with skopeo if you plan to use the output + # nix run nixpkgs#skopeo -- inspect docker-archive:result >/dev/null ... - echo "## Docker Image Analysis (Nix Build - ${{ matrix.arch }})" >> $GITHUB_STEP_SUMMARY + echo "## Docker Image Analysis (Nix Build - ${{ matrix.arch }})" >> "$GITHUB_STEP_SUMMARY" ... - TAGS="${{ needs.build.outputs.image-tags }}" - for tag in $TAGS; do - echo "- \`$tag\`" >> $GITHUB_STEP_SUMMARY + TAGS="${{ needs.build.outputs.image-tags }}" + for tag in $TAGS; do + echo "- \`$tag\`" >> "$GITHUB_STEP_SUMMARY" doneYou can apply the same quoting pattern consistently wherever shellcheck flagged SC2086/SC2129.
Also applies to: 107-125, 126-145, 189-200
107-125: Guard GHCR pushes/manifests for fork PRs and restricted tokensRight now both the per‑arch push step and the multi‑arch manifest creation run unconditionally on
pull_requestandpush. For PRs from forks,GITHUB_TOKENtypically won’t havepackages: write, so:
skopeo login+copytoghcr.iocan fail with permission errors.docker manifest pushin thecreate-manifestjob will also fail.If you don’t need to publish images for fork PRs, consider gating pushes and manifest creation to trusted events, for example:
- name: Push Docker image to registry if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository ... ... - name: Create and push multi-arch manifests if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository run: | ...That keeps CI builds running for all PRs (including forks) without failing on registry permissions, while still publishing images for main/version branches and same‑repo PRs.
Also applies to: 149-187
.nix/dev-shell.nix (2)
21-57: Be cautious about force‑removing an existingcaspertheme directory
ghost-dev-setupunconditionally deletesghost/core/content/themes/casperif the directory exists but isn’t a Git repo:if [ -d "ghost/core/content/themes/casper" ]; then rm -rf ghost/core/content/themes/casper fi git clone --depth 1 ... ghost/core/content/themes/casperIn a dev tree, someone may have a customized or symlinked
caspertheme there; re‑running the setup script would wipe that without warning.Consider at least checking for a Git repo or prompting before deletion, for example:
- Only
rm -rfif it’s not a symlink and either empty or clearly not customized.- Or print a warning and require a
--forceflag/env var before deleting an existing non‑git directory.That keeps the helper script convenient while reducing surprise destruction of local work.
60-98: Verifymysql80/mailpitavailability on aarch64‑darwin dev shellsThe dev shell includes
mysql80,redis,mailpit, and other services unconditionally:packages = with pkgs; [ nodejs_22 yarn sqlite mysql80 redis mailpit … ];Because
flake.nixdeclaresaarch64-darwininsystems, anyone on Apple Silicon macOS will try to build this shell. Ifmysql80ormailpitare unsupported/broken on Darwin in your chosen nixpkgs rev,nix developwill fail entirely.If Darwin support is non‑goal for now, you could either:
- Drop
aarch64-darwinfrom the flake’ssystems, or- Gate Linux‑only tools behind
pkgs.stdenv.isLinuxand provide a lighter, DB‑less shell for macOS.This will avoid surprising breakage for macOS contributors.
flake.nix (1)
25-31: Align declaredsystemswith actual support level
systemsincludes"aarch64-darwin"and exports devShells/apps for it, but some underlying packages (e.g.mysql80,mailpit) may not be available or stable on that platform. Combined with the unconditional use of those tools in.nix/dev-shell.nix, this can lead to failednix developon macOS.If macOS support is still experimental, you might:
- Temporarily drop
"aarch64-darwin"fromsystems, or- Split configs so Darwin gets a reduced devShell that omits Linux‑only services while still exposing linting/formatting/apps.
This will set clearer expectations for contributors about which platforms are first‑class in the Nix setup.
Also applies to: 150-152
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
flake.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.docker-nix/default.nix(1 hunks).docker-nix/etc/group(1 hunks).docker-nix/etc/nsswitch.conf(1 hunks).docker-nix/etc/passwd(1 hunks).envrc(1 hunks).github/workflows/ci-docker-nix.yml(1 hunks).gitignore(1 hunks).nix/README.md(1 hunks).nix/dev-shell.nix(1 hunks).nix/githooks/README.md(1 hunks).nix/githooks/pre-commit(1 hunks).nix/precache-package/default.nix(1 hunks).nix/precache-package/precache-package.sh(1 hunks).nix/process-compose.yaml(1 hunks).nix/treefmt.nix(1 hunks).nix/update-yarn-hash/default.nix(1 hunks).nix/update-yarn-hash/update-yarn-hash.sh(1 hunks)flake.nix(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (13)
- .docker-nix/etc/group
- .gitignore
- .nix/githooks/pre-commit
- .docker-nix/etc/nsswitch.conf
- .nix/update-yarn-hash/update-yarn-hash.sh
- .nix/treefmt.nix
- .nix/precache-package/precache-package.sh
- .nix/update-yarn-hash/default.nix
- .envrc
- .nix/githooks/README.md
- .docker-nix/default.nix
- .docker-nix/etc/passwd
- .nix/precache-package/default.nix
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
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: 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: Run `yarn docker:dev` to start Ghost in Docker with hot reload
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: 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: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:build` to build Docker images and delete ephemeral volumes
📚 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:
.nix/process-compose.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:
.nix/process-compose.yaml.nix/dev-shell.nix
📚 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:
.nix/process-compose.yaml
📚 Learning: 2025-06-19T22:57:05.880Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23941
File: .github/workflows/ci.yml:911-914
Timestamp: 2025-06-19T22:57:05.880Z
Learning: In Ghost, when NODE_ENV is set to testing-mysql, Ghost uses config.testing-mysql.json configuration which sets the server port to 2369 instead of the default 2368. This also applies to other testing environments like testing and testing-browser.
Applied to files:
.nix/process-compose.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:
.nix/process-compose.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:
.nix/process-compose.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:
.nix/process-compose.yaml.nix/dev-shell.nix
📚 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:
.nix/process-compose.yaml.nix/dev-shell.nix
📚 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:
.nix/process-compose.yaml
🪛 actionlint (1.7.9)
.github/workflows/ci-docker-nix.yml
46-46: shellcheck reported issue in this script: SC2086:info:13:29: Double quote to prevent globbing and word splitting
(shellcheck)
46-46: shellcheck reported issue in this script: SC2086:info:9:28: Double quote to prevent globbing and word splitting
(shellcheck)
110-110: shellcheck reported issue in this script: SC2086:info:13:14: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2034:warning:4:1: IMAGE_INFO appears unused. Verify use (or export if used externally)
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:10:69: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:11:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:12:37: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:13:48: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:14:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:15:49: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:16:12: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2086:info:17:60: Double quote to prevent globbing and word splitting
(shellcheck)
127-127: shellcheck reported issue in this script: SC2129:style:10:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:10:83: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:1:50: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:3:63: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:4:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:7:24: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2086:info:9:12: Double quote to prevent globbing and word splitting
(shellcheck)
190-190: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
🪛 LanguageTool
.nix/README.md
[uncategorized] ~94-~94: The official name of this software platform is spelled with a capital “H”.
Context: ...cache-package/precache-package.sh, and .github/workflows/ci-docker-nix.yml`. Cachix is...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
.nix/README.md
23-23: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (2)
.nix/README.md (1)
1-96: Well-structured and informative Nix infrastructure documentation.The README clearly explains the value proposition, measured benefits, developer workflow, and setup instructions. The FAQ addresses key concerns (determinism, maintainability, production readiness). The tone balances technical depth with accessibility.
A few observations:
- The binary cache note (line 92–94) appropriately flags that the personal
hello-stochaCachix cache is temporary and explains the transition path for production adoption.- The "5-minute dev setup" (lines 40–56) is clear and lowers the barrier to entry for developers unfamiliar with Nix.
- Requirements section correctly lists Nix flakes, Cachix auth token, and cost expectations.
flake.nix (1)
127-133: Unable to complete verification due to repository access limitationsThe review comment cannot be definitively verified or refuted without access to the actual
flake.nixand.nix/process-compose.yamlfiles in this repository. However, based on the learnings context provided—which describes Ghost's development environment using Docker Compose with containerized services (MySQL, Redis, Mailpit, Caddy)—the assumption underlying this review comment may need reassessment.The comment assumes
process-compose.yamldirectly executesmysqld,redis-server,mailpit, and other binaries on the host machine. If services are instead containerized or delegated to Docker, the suggested fix (adding nodejs_22, mysql80, redis, mailpit to runtimeInputs) may not address the actual architecture.To properly verify this comment, one would need to:
- Inspect
.nix/process-compose.yamlto confirm whether services run as local processes or containers- Check the dev shell configuration to see if tools are already provided
- Determine if
nix run .#devis actually broken or if the architecture differs from the comment's assumptions
onethirtyfive
force-pushed
the
feat/nix-docker-builds
branch
2 times, most recently
from
172442f to
1729211
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.nix/precache-package/precache-package.sh (1)
20-20: Fix array expansion with strict mode—unresolved issue from prior review.This was previously flagged by cursor[bot]: The script uses
set -u(unbound variable checking) but expands potentially empty arrays with${ARGS[@]}(line 20) and${NIX_EXTRA_ARGS[@]}(line 126). When these arrays are empty, the script fails. The portable fix is to use parameter expansion with a default:"${ARGS[@]:-}"or"${ARGS[@]+"${ARGS[@]}"}"Apply this diff to fix both locations:
-set -- "${ARGS[@]}" +set -- "${ARGS[@]:-}"BUILD_RESULT=$(cachix watch-exec "$CACHE_NAME" -- nix build \ "$FLAKE_URL#$OUTPUT" \ --print-out-paths \ --no-link \ - "${NIX_EXTRA_ARGS[@]}") + "${NIX_EXTRA_ARGS[@]:-}")Also applies to: 126-126
.github/workflows/ci-docker-nix.yml (1)
44-59: Fork PR issue has been addressed—local flake references are now in place.The previous review flagged that remote flake references (
github:${{ github.repository }}/$SHA) fail for fork PRs because the fork commit doesn't exist in the base repo. The current code correctly uses local flake references (.#) at both the cache-check stage (line 48) and build stage (line 83), which ensures the evaluated flake is against the checked-out repository contents regardless of whether the PR is from a fork or the main repo.However, verify that the cache-check logic at lines 51–52 (querying remote Cachix for path availability) behaves as expected for fork PRs, where the build may not have been precached in Cachix.
Also applies to: 74-86
🧹 Nitpick comments (7)
.nix/README.md (2)
77-86: Add language specifier to fenced code block.Static analysis flagged this code block as missing a language specifier. For plain text/file structure listings, use
```textor```plaintext.-``` +```text flake.nix # Package definitions .docker-nix/default.nix # Docker image build .nix/
21-27: Consider using a proper heading for the "Measured Results" subsection.Line 23 uses bold emphasis for a section title. For consistent document structure and accessibility, a heading (
###) may be preferable.## Measured Results -**Build time with warm cache: 15-20 min → 3-6 min (70% faster)** +### Build time with warm cache: 15-20 min → 3-6 min (70% faster).nix/process-compose.yaml (2)
64-71: Consider using standard Redis PING for readiness check.The current check
redis-cli --raw incr pingincrements a counter key named "ping" rather than using the standardPINGcommand. While functional, it's unconventional and creates a side effect.readiness_probe: exec: - command: "redis-cli --raw incr ping" + command: "redis-cli ping"
20-28: Large InnoDB buffer pool may strain some dev machines.
innodb-buffer-pool-size=1Gallocates significant memory. This is fine for most dev setups, but consider documenting this or making it configurable for resource-constrained environments..nix/update-yarn-hash/update-yarn-hash.sh (1)
57-64: Hash extraction pipeline is somewhat fragile.The grep/sed chain assumes a specific format in the Nix file. If the file structure changes (e.g., multiple
fetchYarnDepscalls or different formatting), this could break silently.Consider adding a more specific pattern or validation:
-CURRENT_HASH=$(grep 'fetchYarnDeps' -A5 "$HASH_FILE" | grep 'hash = "' | sed 's/.*hash = "\([^"]*\)".*/\1/' | head -1) +# Extract hash from yarn-offline-cache block specifically +CURRENT_HASH=$(grep -A5 'yarn-offline-cache = pkgs.fetchYarnDeps' "$HASH_FILE" | grep 'hash = "' | sed 's/.*hash = "\([^"]*\)".*/\1/' | head -1).github/workflows/ci-docker-nix.yml (2)
114-122: Quote variables in loops to handle tags with spaces or special characters safely.The loop variable
$tagshould be quoted in thedocker://$tag-...argument to prevent word splitting if a tag contains spaces or special characters.Apply this diff to improve shell quoting safety:
TAGS="${{ steps.meta.outputs.tags }}" for tag in $TAGS; do echo "Pushing: $tag-${{ matrix.arch }}" nix run nixpkgs#skopeo -- copy \ --preserve-digests \ docker-archive:result \ docker://"$tag"-${{ matrix.arch }} done
187-199: Consolidate step summary writes using a grouped block (SC2129).Similar to the inspect image step, the summary writes can be grouped into a single append block for clarity and efficiency.
Apply this diff:
- name: Summary run: | - echo "## Multi-Architecture Images Published" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "The following images support both x86_64 and ARM64:" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - TAGS="${{ needs.build.outputs.image-tags }}" - for tag in $TAGS; do - echo "- \`$tag\`" >> $GITHUB_STEP_SUMMARY - done - echo "" >> $GITHUB_STEP_SUMMARY - echo "Docker will automatically select the correct architecture when pulling." >> $GITHUB_STEP_SUMMARY + TAGS="${{ needs.build.outputs.image-tags }}" + { + echo "## Multi-Architecture Images Published" + echo "" + echo "The following images support both x86_64 and ARM64:" + echo "" + for tag in $TAGS; do + echo "- \`$tag\`" + done + echo "" + echo "Docker will automatically select the correct architecture when pulling." + } >> $GITHUB_STEP_SUMMARY
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
flake.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
.docker-nix/default.nix(1 hunks).docker-nix/etc/group(1 hunks).docker-nix/etc/nsswitch.conf(1 hunks).docker-nix/etc/passwd(1 hunks).envrc(1 hunks).github/workflows/ci-docker-nix.yml(1 hunks).gitignore(1 hunks).nix/README.md(1 hunks).nix/dev-shell.nix(1 hunks).nix/githooks/README.md(1 hunks).nix/githooks/pre-commit(1 hunks).nix/precache-package/default.nix(1 hunks).nix/precache-package/precache-package.sh(1 hunks).nix/process-compose.yaml(1 hunks).nix/treefmt.nix(1 hunks).nix/update-yarn-hash/default.nix(1 hunks).nix/update-yarn-hash/update-yarn-hash.sh(1 hunks)flake.nix(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (10)
- .docker-nix/etc/passwd
- .nix/update-yarn-hash/default.nix
- .nix/precache-package/default.nix
- .nix/githooks/README.md
- .docker-nix/etc/group
- .envrc
- .nix/githooks/pre-commit
- flake.nix
- .gitignore
- .nix/dev-shell.nix
🧰 Additional context used
🧠 Learnings (14)
📓 Common learnings
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: 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
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: 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: 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: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:build` to build Docker images and delete ephemeral volumes
📚 Learning: 2025-04-23T04:01:25.459Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 22996
File: package.json:40-40
Timestamp: 2025-04-23T04:01:25.459Z
Learning: The xargs `-r` flag is not supported in standard BSD xargs (default on macOS) and will produce an error. However, macOS users may have GNU xargs installed (via Homebrew/MacPorts) which does support the `-r` flag. For cross-platform compatibility, using the `-I{}` placeholder approach is safer.
Applied to files:
.nix/update-yarn-hash/update-yarn-hash.sh
📚 Learning: 2025-09-10T21:24:49.363Z
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.
Applied to files:
.github/workflows/ci-docker-nix.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:
.docker-nix/default.nix.nix/process-compose.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:
.docker-nix/default.nix.nix/process-compose.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:
.docker-nix/default.nix.nix/process-compose.yaml
📚 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:
.docker-nix/default.nix.nix/process-compose.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:
.docker-nix/default.nix.nix/process-compose.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: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
Applied to files:
.docker-nix/default.nix
📚 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:build` to build Docker images and delete ephemeral volumes
Applied to files:
.docker-nix/default.nix
📚 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:
.docker-nix/default.nix.nix/process-compose.yaml
📚 Learning: 2025-06-19T22:57:05.880Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 23941
File: .github/workflows/ci.yml:911-914
Timestamp: 2025-06-19T22:57:05.880Z
Learning: In Ghost, when NODE_ENV is set to testing-mysql, Ghost uses config.testing-mysql.json configuration which sets the server port to 2369 instead of the default 2368. This also applies to other testing environments like testing and testing-browser.
Applied to files:
.nix/process-compose.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:
.nix/process-compose.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:
.nix/process-compose.yaml
🪛 actionlint (1.7.9)
.github/workflows/ci-docker-nix.yml
46-46: shellcheck reported issue in this script: SC2086:info:11:29: Double quote to prevent globbing and word splitting
(shellcheck)
46-46: shellcheck reported issue in this script: SC2086:info:7:28: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:13:14: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2034:warning:4:1: IMAGE_INFO appears unused. Verify use (or export if used externally)
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:10:69: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:11:12: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:12:37: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:13:48: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:14:12: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:15:49: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:16:12: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:17:60: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2129:style:10:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
188-188: shellcheck reported issue in this script: SC2086:info:10:83: Double quote to prevent globbing and word splitting
(shellcheck)
188-188: shellcheck reported issue in this script: SC2086:info:1:50: Double quote to prevent globbing and word splitting
(shellcheck)
188-188: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
188-188: shellcheck reported issue in this script: SC2086:info:3:63: Double quote to prevent globbing and word splitting
(shellcheck)
188-188: shellcheck reported issue in this script: SC2086:info:4:12: Double quote to prevent globbing and word splitting
(shellcheck)
188-188: shellcheck reported issue in this script: SC2086:info:7:24: Double quote to prevent globbing and word splitting
(shellcheck)
188-188: shellcheck reported issue in this script: SC2086:info:9:12: Double quote to prevent globbing and word splitting
(shellcheck)
188-188: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
🪛 LanguageTool
.nix/README.md
[uncategorized] ~94-~94: The official name of this software platform is spelled with a capital “H”.
Context: ...cache-package/precache-package.sh, and .github/workflows/ci-docker-nix.yml`. Cachix is...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
.nix/README.md
23-23: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (13)
.nix/precache-package/precache-package.sh (2)
22-66: Excellent usage documentation and clear variable setup.The usage message comprehensively explains the script's purpose, arguments, options, and provides practical examples for local and remote precaching workflows. Variable handling and CACHE_NAME defaults are clean.
68-131: Solid defensive programming and cache management logic.Environment validation, path-based URL guarding against uncommitted changes, and cache existence checking are well-designed. Using
nix evalto determine the output path without building, then checking the remote cachix store, is a smart approach. Thecachix watch-execintegration properly primes the full dependency closure..nix/treefmt.nix (1)
1-13: Treefmt config is well‑structured and aligned with flake-based layoutThis module cleanly wires treefmt-nix: it anchors on
flake.nix, enables sane default excludes, and configuresnixfmt-rfc-stylewith sensible include globs for Nix files. I don’t see any issues; this should integrate nicely with the flake’s formatter outputs.If you bump
treefmt-nixornixpkgslater, please double-check thatenableDefaultExcludes,projectRootFile, andprograms.nixfmt.*still match the current module option names in the treefmt-nix docs..docker-nix/etc/nsswitch.conf (1)
1-3: LGTM!Standard minimal NSS configuration for container environments. The
files dnsorder for hosts resolution is correct, ensuring local/etc/hostsentries are checked before DNS queries..docker-nix/default.nix (4)
166-176: Install phase conditionals are correct but could be more explicit.The
[ -d dir ] && cp -r dir $out/pattern works, but if a workspace produces none of these directories, the install phase still succeeds (which may or may not be intended). This is fine for flexibility across different workspace types.
384-421: Development image configuration looks correct.The Docker image is properly configured for development use with
NODE_ENV=development, NX daemon enabled, and appropriate environment variables. TheWorkingDirandCmdalign with Ghost's dev workflow.
83-86: Hash managed by update-yarn-hash script.The hardcoded
fetchYarnDepshash is expected and managed by thenix run .#update-yarn-hashtooling. The pre-commit hook validates this stays in sync withyarn.lock.
111-118: Native module rebuilds and cleanup are well-structured.Rebuilding sqlite3, sharp, and re2 from source against Nix libraries is necessary for sandbox compatibility. The cleanup of
.o,.a, andobj.targetartifacts reduces image size effectively..nix/update-yarn-hash/update-yarn-hash.sh (3)
8-17: Portable sed helper addresses cross-platform compatibility.The
sed_inplacefunction correctly detects GNU vs BSD sed and applies the appropriate-isyntax. This resolves the previously flagged macOS compatibility issue.
81-84: Build output capture approach is effective.Using a temp file with proper cleanup trap to capture the intentional build failure output is a reliable pattern for extracting the correct hash from Nix's error message.
90-99: Good error handling with rollback.If hash extraction fails, the script correctly restores the original hash before exiting. This prevents leaving the file in a broken state.
.nix/process-compose.yaml (1)
41-55: This concern is unfounded—knex-migrator init creates the database automatically.The
knex-migrator initcommand creates theghostdatabase if it doesn't exist, provided the database user has CREATE privileges. The MySQL root user (configured in the environment) has full privileges and will successfully create the database. No explicitCREATE DATABASEstep is needed.Likely an incorrect or invalid review comment.
.github/workflows/ci-docker-nix.yml (1)
162-185: Verify thatdocker manifestcommands work against GHCR in the Actions environment.The manifest creation job uses standard
docker manifestcommands after logging in to GHCR. Ensure that the Actions environment has Docker CLI with manifest support enabled (which it should by default in GitHub Actions) and that thedocker/login-action@v3step correctly sets up auth for manifest operations.
| "NX_DAEMON=true" | ||
| "GHOST_DEV_IS_DOCKER=true" | ||
| "LD_LIBRARY_PATH=${commonEnv.LD_LIBRARY_PATH}" | ||
| "PATH=/home/ghost/node_modules/.bin:/usr/bin:/bin:${nodejs}/bin:${pkgs.yarn}/bin:${pkgs.stripe-cli}/bin" |
There was a problem hiding this comment.
Bug: Docker image PATH missing Nix store paths for utilities
The Docker image's PATH environment variable is missing the Nix store paths for several packages included in contents: pkgs.bash, pkgs.coreutils, pkgs.findutils, pkgs.gnugrep, pkgs.which, and pkgs.procps. In Nix Docker images, packages install their binaries to Nix store paths (e.g., /nix/store/xxx-bash/bin/bash), not to /bin or /usr/bin. Since these paths aren't in PATH, basic commands like ls, cat, bash, find, grep, which, and ps won't be available, making the container difficult to use interactively and potentially causing failures in scripts that depend on these utilities.
|
Curious if there's any interest in this changeset or the ideas behind it. Happy to close it if not. |
no issue Provides an alternative build system with reproducible, content-addressed caching. Measured results: 70% faster CI with warm cache (15-20min → 3-6min). Native dev environment eliminates Docker Desktop overhead on macOS. Runs in parallel with existing CI: additive, zero risk to evaluate.
no issue Nix requires knowing dependency hashes upfront for reproducibility. When yarn.lock changes, the hash in .docker-nix/default.nix must be updated. - `nix run .#update-yarn-hash` automates hash discovery and replacement - Pre-commit hook validates hash when yarn.lock is staged (~4s)
no issue Hash was stale after upstream dependency changes. Regenerated using `nix run .#update-yarn-hash`. Short, explains what and why, follows their format.
onethirtyfive
force-pushed
the
feat/nix-docker-builds
branch
from
1729211 to
037d684
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.nix/README.md (1)
77-86: Add language specifier to fenced code block.The file structure code block should have a language specifier for better rendering. Use
textorplaintextfor non-executable content.-``` +```text flake.nix # Package definitions .docker-nix/default.nix # Docker image build .nix/ ├── dev-shell.nix # Development environment ├── update-yarn-hash/ # Hash update automation ├── precache-package/ # CI cache warming ├── githooks/ # Pre-commit validation └── process-compose.yaml # Local service orchestration -``` +```.nix/update-yarn-hash/update-yarn-hash.sh (1)
19-42: Consider auto-detecting the default system.The default system is hardcoded to
aarch64-linux(line 20), but developers on x86_64 machines would need to explicitly pass the argument. Consider detecting the system automatically like the pre-commit hook does, while still allowing the override.# Parse arguments -SYSTEM="${1:-aarch64-linux}" +if [[ -z "${1:-}" ]]; then + SYSTEM=$(nix eval --raw --expr builtins.currentSystem 2>/dev/null || echo "aarch64-linux") +else + SYSTEM="$1" +fi.github/workflows/ci-docker-nix.yml (2)
114-122: Quote variables in loop to prevent word splitting issues.The
TAGSvariable should be quoted in the loop iteration to handle tags with spaces correctly (though unlikely with Docker tags). This addresses the SC2086 shellcheck warning.# Push with all tags, appending arch suffix TAGS="${{ steps.meta.outputs.tags }}" - for tag in $TAGS; do + while IFS= read -r tag; do + [ -z "$tag" ] && continue echo "Pushing: $tag-${{ matrix.arch }}" nix run nixpkgs#skopeo -- copy \ --preserve-digests \ docker-archive:result \ - docker://$tag-${{ matrix.arch }} - done + "docker://$tag-${{ matrix.arch }}" + done <<< "$TAGS"
186-197: Consolidate summary writes into a grouped block.Per static analysis hint SC2129, consolidate the repeated
>> $GITHUB_STEP_SUMMARYappends into a single block for efficiency and readability.- name: Summary run: | - echo "## Multi-Architecture Images Published" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY - echo "The following images support both x86_64 and ARM64:" >> $GITHUB_STEP_SUMMARY - echo "" >> $GITHUB_STEP_SUMMARY TAGS="${{ needs.build.outputs.image-tags }}" - for tag in $TAGS; do - echo "- \`$tag\`" >> $GITHUB_STEP_SUMMARY - done - echo "" >> $GITHUB_STEP_SUMMARY - echo "Docker will automatically select the correct architecture when pulling." >> $GITHUB_STEP_SUMMARY + { + echo "## Multi-Architecture Images Published" + echo "" + echo "The following images support both x86_64 and ARM64:" + echo "" + while IFS= read -r tag; do + [ -z "$tag" ] && continue + echo "- \`$tag\`" + done <<< "$TAGS" + echo "" + echo "Docker will automatically select the correct architecture when pulling." + } >> "$GITHUB_STEP_SUMMARY"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
flake.lockis excluded by!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (18)
.docker-nix/default.nix(1 hunks).docker-nix/etc/group(1 hunks).docker-nix/etc/nsswitch.conf(1 hunks).docker-nix/etc/passwd(1 hunks).envrc(1 hunks).github/workflows/ci-docker-nix.yml(1 hunks).gitignore(1 hunks).nix/README.md(1 hunks).nix/dev-shell.nix(1 hunks).nix/githooks/README.md(1 hunks).nix/githooks/pre-commit(1 hunks).nix/precache-package/default.nix(1 hunks).nix/precache-package/precache-package.sh(1 hunks).nix/process-compose.yaml(1 hunks).nix/treefmt.nix(1 hunks).nix/update-yarn-hash/default.nix(1 hunks).nix/update-yarn-hash/update-yarn-hash.sh(1 hunks)flake.nix(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .nix/precache-package/default.nix
🚧 Files skipped from review as they are similar to previous changes (10)
- .docker-nix/etc/passwd
- .gitignore
- .docker-nix/etc/nsswitch.conf
- .nix/dev-shell.nix
- .nix/treefmt.nix
- .nix/process-compose.yaml
- flake.nix
- .nix/precache-package/precache-package.sh
- .envrc
- .docker-nix/etc/group
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
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
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: 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: 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: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
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
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: CR
Repo: TryGhost/Ghost PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T14:28:50.351Z
Learning: Run `yarn docker:build` to build Docker images and delete ephemeral volumes
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.
📚 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: Always use `yarn` (v1) for all commands in this Yarn v1 + Nx monorepo
Applied to files:
.nix/update-yarn-hash/default.nix
📚 Learning: 2025-04-23T04:01:25.459Z
Learnt from: cmraible
Repo: TryGhost/Ghost PR: 22996
File: package.json:40-40
Timestamp: 2025-04-23T04:01:25.459Z
Learning: The xargs `-r` flag is not supported in standard BSD xargs (default on macOS) and will produce an error. However, macOS users may have GNU xargs installed (via Homebrew/MacPorts) which does support the `-r` flag. For cross-platform compatibility, using the `-I{}` placeholder approach is safer.
Applied to files:
.nix/update-yarn-hash/update-yarn-hash.sh
📚 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:
.docker-nix/default.nix
📚 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:
.docker-nix/default.nix
📚 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:
.docker-nix/default.nix
📚 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:
.docker-nix/default.nix
📚 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:
.docker-nix/default.nix
📚 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: Admin-x React apps build to `apps/*/dist` using Vite, which are then copied by `ghost/admin/lib/asset-delivery` to `ghost/core/core/built/admin/assets/*`
Applied to files:
.docker-nix/default.nix
📚 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:build` to build Docker images and delete ephemeral volumes
Applied to files:
.docker-nix/default.nix
📚 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:
.docker-nix/default.nix
📚 Learning: 2025-09-10T21:24:49.363Z
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.
Applied to files:
.github/workflows/ci-docker-nix.yml
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
.github/workflows/ci-docker-nix.yml
🪛 actionlint (1.7.9)
.github/workflows/ci-docker-nix.yml
46-46: shellcheck reported issue in this script: SC2086:info:11:29: Double quote to prevent globbing and word splitting
(shellcheck)
46-46: shellcheck reported issue in this script: SC2086:info:7:28: Double quote to prevent globbing and word splitting
(shellcheck)
108-108: shellcheck reported issue in this script: SC2086:info:13:14: Double quote to prevent globbing and word splitting
(shellcheck)
125-125: shellcheck reported issue in this script: SC2086:info:16:6: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2086:info:10:83: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2086:info:1:50: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2086:info:2:12: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2086:info:3:63: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2086:info:4:12: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2086:info:7:24: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2086:info:9:12: Double quote to prevent globbing and word splitting
(shellcheck)
187-187: shellcheck reported issue in this script: SC2129:style:1:1: Consider using { cmd1; cmd2; } >> file instead of individual redirects
(shellcheck)
🪛 LanguageTool
.nix/README.md
[uncategorized] ~94-~94: The official name of this software platform is spelled with a capital “H”.
Context: ...cache-package/precache-package.sh, and .github/workflows/ci-docker-nix.yml`. Cachix is...
(GITHUB)
🪛 markdownlint-cli2 (0.18.1)
.nix/README.md
23-23: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
77-77: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (10)
.nix/README.md (1)
1-108: Well-structured documentation for the Nix infrastructure.The README provides comprehensive coverage of the Nix build system benefits, measured results, developer workflows, and FAQ. The acknowledgment of the temporary Cachix cache name and transition path is helpful for future adoption.
.nix/githooks/README.md (1)
1-28: Clear and concise hook documentation.The documentation accurately explains the purpose, installation options, and behavior of the pre-commit hook, including the graceful skip when Nix isn't installed.
.nix/githooks/pre-commit (1)
1-54: Well-implemented pre-commit hook with proper cross-platform support.The script correctly addresses the previously flagged architecture issue by dynamically detecting the system. The graceful skip when Nix isn't installed ensures developers without Nix aren't blocked. Good use of strict bash mode.
.nix/update-yarn-hash/default.nix (1)
1-13: Clean Nix derivation for the update-yarn-hash tool.Idiomatic use of
writeShellApplicationwith appropriate runtime dependencies. This pattern ensures the script has access tonix,sed, andgrepregardless of the host environment..nix/update-yarn-hash/update-yarn-hash.sh (1)
1-123: Well-structured hash update script with proper error handling.The script correctly handles the fake hash trick for discovering the correct hash, restores the original on failure, and verifies the build succeeds after updating. The
sed_inplacehelper properly addresses the macOS compatibility issue from previous reviews..github/workflows/ci-docker-nix.yml (1)
1-206: Well-structured multi-arch CI workflow.The workflow correctly handles both x86_64 and aarch64 builds with matrix strategy, uses Cachix for binary caching, and creates proper multi-arch manifests. The previous fork PR issue has been addressed by using the local flake reference.
.docker-nix/default.nix (4)
105-121: Native module rebuild approach is solid.The explicit rebuild of sqlite3, sharp, and re2 native modules using node-gyp ensures they're compiled against Nix libraries rather than using prebuilt binaries. Cleaning up
.o,.a, andobj.targetartifacts helps reduce image size.
139-177: Clean abstraction for workspace builds.The
mkWorkspaceBuildhelper provides a reusable pattern for building workspace packages with dependency injection viaextraDeps. This reduces duplication across the individual builders.
335-363: Ghost app assembly correctly combines all build artifacts.The
ghost-appderivation properly assembles outputs from all individual builders into the final application structure, matching the expected directory layout.
388-401: PATH configuration is correct for the contents list.The
contentsparameter inpkgs.dockerTools.buildLayeredImageautomatically symlinks all included packages into the image root. This meanspkgs.bash,pkgs.coreutils,pkgs.findutils,pkgs.gnugrep,pkgs.which, andpkgs.procpsbecome available at/bin/*inside the container without requiring explicit Nix store paths inPATH. The currentPATH=/usr/bin:/binis appropriate and sufficient for these utilities.Likely an incorrect or invalid review comment.
1 participant
Context
I'm currently looking for work and Ghost caught my attention—I really like the product and the FOSS ethos behind it. I took it for a test drive, got curious about the build system and developer environment setup, and thought: "I bet I could make this faster and more reliable with Nix."
So I did! This is the result.
Summary
Additive support for Nix-based Docker builds and native, non-containerized multi-architecture developer environments for Ghost. (The native and containerized Ghost packages are bit-identical.)
What this provides
Reproducible builds. Every input is locked: source, dependencies, and toolchain. A build that succeeds today will succeed tomorrow, next month, next year. No build surprises from upstream externalities, ever.
Global, content-addressed caching. Same inputs produce the same hash, which hits the same cache—regardless of branch, PR, or machine. A build anywhere warms the cache for everyone. Devs can even precache CI builds on their beefy work laptops.
70% faster CI with warm cache:
node_modulescontents: 3-6 min.Full disclosure: the first build of any novel set of node dependencies will be CPU-bound by its runner--quite slow on github runners, much faster on beefy self-hosted runners or, even better, on a dev's laptop and pushed to the cache.
At scale, this pays for itself. Hundreds of builds per week × minutes saved per build × engineers waiting or context-switching adds up to real salary-equivalents in recovered productivity. Enough to fund another developer (ahem!), or just stop losing Friday afternoons to CI mysteries.
Native dev environment (no Docker required)
The same expressions that build the Docker image define a native dev environment. MySQL, Redis, Ghost—running as native processes, not containers. 2-3 second startup, no VM overhead, no Docker Desktop eating 12GB of RAM on macOS. Guaranteed to match what CI builds.
Here's what you see when you run
ghost-dev, a thin script which callsprocess-composeto spin up native processes needed for ghost without the overhead of containerization:More information and context in
.nix/README.md.Conclusion
This is a working proof-of-concept: faster builds, reproducible outputs, native dev environments—all additive to your existing workflow.
If there's interest, I'm happy to walk through the implementation, answer questions, or help with adoption. And if not, no hard feelings—it was a good excuse to learn your codebase.
Note
Introduce Nix flake-based build system that produces a Docker image and native dev environment, with CI publishing multi-arch images and tooling for caching and yarn hash management.
.docker-nix/default.nix(compiles native node modules, prepares runtime, builds app artifacts, and assembles layered image).flake.nix/flake.lockand filteredsourcederivation; expose packages (e.g.,dockerImage, builders)..docker-nix/etc/*..github/workflows/ci-docker-nix.ymlto build with Nix, leverage Cachix cache, push arch-suffixed images to GHCR, and create multi-arch manifests..nix/dev-shell.nixwithghost-dev(process-compose MySQL/Redis/Mailpit/Ghost) and setup helper, plus.envrcfor direnv..nix/process-compose.yaml..nix/githooks/*,update-yarn-hash,precache-package..nix/treefmt.nixand docs.nix/README.md..gitignorefor Nix/direnv artifacts and dev data.Written by Cursor Bugbot for commit 037d684. This will update automatically on new commits. Configure here.