Changed devcontainer dev stack to match Admin + Portal default#29106
Changed devcontainer dev stack to match Admin + Portal default#291069larsons wants to merge 10 commits into
Conversation
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR updates devcontainer and dev-gateway setup so public apps are served from built Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin-x-settings:test:acceptance |
✅ Succeeded | 9m 53s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 45s | View ↗ |
nx run ghost:test:ci:integration |
✅ Succeeded | 3s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 55s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 26s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | <1s | View ↗ |
nx run @tryghost/comments-ui:test:acceptance |
✅ Succeeded | 46s | View ↗ |
nx run @tryghost/activitypub:test:acceptance |
✅ Succeeded | 45s | View ↗ |
Additional runs (7) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-05 00:37:35 UTC
9larsons
force-pushed
the
codex/public-dev-opt-ins-main
branch
3 times, most recently
from
84a1525 to
7f788ce
Compare
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
no ref pnpm dev's default frontend fan-out was narrowed to Admin + Portal only (#29104/#29105), since most sessions never touch the public UMD apps and those watchers are the heaviest processes in the stack. The devcontainer's start-dev-stack.sh hardcoded the old full project list independently of root pnpm dev, so it needed the same trim to actually get the memory savings inside Codespaces/devcontainers. Public apps remain available via an explicit nx run-many invocation, called out in the script's output.
no ref #28970 switched public apps (Portal, Comments UI, Signup Form, Sodo Search, Announcement Bar, Admin Toolbar) from a proxied vite preview server to Caddy serving their umd/ build output directly via file_server, but left the now-unread *_DEV_SERVER env vars in the gateway Dockerfile and the devcontainer's compose override, and left their ports forwarded in devcontainer.json. Confirmed via grep that the Caddyfile only ever reads GHOST_BACKEND, ADMIN_DEV_SERVER, ADMIN_LIVE_RELOAD_SERVER, LEXICAL_DEV_SERVER, ANALYTICS_PROXY_TARGET, and ACTIVITYPUB_PROXY_TARGET. Fixing at the Dockerfile (shared by host and devcontainer builds) removes the need to keep re-declaring dead vars in the devcontainer overlay, and stops Codespaces from showing five phantom ports nothing ever binds.
9larsons
force-pushed
the
slars/devcontainer-ghost-portal-default
branch
from
32009eb to
f49dfd3
Compare
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
no ref apps/admin's dev target dependsOn ghost-monorepo:docker:up (added by #29101 so host pnpm dev doesn't race Vite against Docker startup). On host that's a harmless no-op re-run, since pnpm dev is a single nx invocation that already deduped docker:up to one execution. Inside the devcontainer, start-dev-stack.sh calls nx run-many for Admin/Portal as a separate invocation from the backend's pnpm --filter ghost dev, so docker:up actually re-executes there. --force-recreate, run from inside the ghost-dev container via the mounted host docker socket, recreates the very container issuing the command -- so Admin's Vite server got stuck behind a docker compose command tearing down its own host, and /ghost/ 502'd indefinitely while Portal (no such dependency) loaded fine. Guarded the actual recreate on CODESPACES/REMOTE_CONTAINERS, which Codespaces and VS Code Dev Containers both set inside the container -- the devcontainer's own compose lifecycle already brings these services up before start-dev-stack.sh ever runs, so the recreate is redundant there regardless of the invocation-count issue.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #29106 +/- ##
==========================================
+ Coverage 74.27% 74.30% +0.02%
==========================================
Files 1565 1565
Lines 135716 135716
Branches 16453 16458 +5
==========================================
+ Hits 100804 100840 +36
+ Misses 33883 33876 -7
+ Partials 1029 1000 -29
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
no ref onCreateCommand and postCreateCommand both ran a full pnpm install --prefer-offline over the same fully-mounted workspace, back to back, with nothing but a submodule update between them. Checked .gitmodules -- both submodules are theme content (Casper, Source), not workspace packages, so there's no dependency-order reason postCreateCommand's install needed the submodules in place first. Dropped the redundant one; onCreateCommand's already covers it.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
…m store no ref .github/workflows/devcontainer-build.yml has published ghcr.io/tryghost/ghost-devcontainer:latest with GHA layer caching on every relevant main push, but nothing consumed it -- the devcontainer's ghost-dev service only had a build: block, so every fresh codespace/container rebuilt the image from scratch (apt-get, backend-only pnpm install) regardless. Considered baking the full-workspace pnpm install into a layered image on top of that base, but compose.devcontainer.yaml bind-mounts the entire repo over /workspaces/Ghost, which fully shadows anything an image bakes in at that path with the host's (node_modules-less) checkout -- the same reason onCreateCommand/postCreateCommand have to run pnpm install live in the first place. A per-workspace-package volume mount could work around that but means ~19 entries to maintain as packages come and go. Instead: added image: pointing at the published tag (with build.cache_from so a local rebuild, e.g. after a Dockerfile edit, still reuses those layers instead of starting cold) and an initializeCommand to pre-pull it into the local image cache before the container is built or pulled. Separately, cached pnpm's content-addressable store as a named volume at /pnpm-store (outside /workspaces/Ghost, so the bind mount can't shadow it) via npm_config_store_dir. The live pnpm install onCreateCommand/postCreateCommand still run every time, but against a warm store they're linking already-fetched content instead of re-downloading and recompiling native deps (better-sqlite3, sharp, etc.) from scratch.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.devcontainer/compose.devcontainer.yaml:
- Around line 34-35: The devcontainer environment is using the wrong store-path
variable for pnpm, so the cache volume will be ignored. In the devcontainer
compose config, update the environment entry under the devcontainer service from
npm_config_store_dir to pnpm_config_store_dir so pnpm@11.8.0 in the
packageManager field can read it correctly. Keep the value pointing to
/pnpm-store and ensure the setting is applied in the same environment block.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a1de04bd-7d32-4953-93ee-6ec3336fffc5
📒 Files selected for processing (2)
.devcontainer/compose.devcontainer.yaml.devcontainer/devcontainer.json
🚧 Files skipped from review as they are similar to previous changes (1)
- .devcontainer/devcontainer.json
| environment: | ||
| npm_config_store_dir: /pnpm-store |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package.json packageManager ==\n'
if [ -f package.json ]; then
grep -n '"packageManager"' package.json || true
fi
printf '\n== repo files likely relevant ==\n'
git ls-files | rg '(^|/)(package.json|pnpm-lock\.yaml|pnpm-workspace\.yaml|\.devcontainer/compose\.devcontainer\.yaml)$' || true
printf '\n== inspect package.json if present ==\n'
if [ -f package.json ]; then
cat -n package.json | sed -n '1,120p'
fi
printf '\n== inspect devcontainer file ==\n'
cat -n .devcontainer/compose.devcontainer.yaml | sed -n '1,120p'Repository: TryGhost/Ghost
Length of output: 13418
🌐 Web query:
pnpm 11.8.0 environment variable config prefixes npm_config_store_dir pnpm_config_store_dir store-dir docs
💡 Result:
In pnpm 11.8.0, configuration is managed through environment variables prefixed with pnpm_config_ or PNPM_CONFIG_ [1][2]. The documentation explicitly states that pnpm no longer reads npm_config_* environment variables [2]. Users are advised to transition to using the pnpm_config_ prefix for all configuration settings [2]. For setting the package store directory, you should use the store-dir configuration setting [3]. You can configure this using the pnpm_config_store_dir environment variable [1][2]. Summary of Key Points: - Environment Variable Prefix: Use pnpm_config_ (e.g., pnpm_config_store_dir) [1][2]. The legacy npm_config_ prefix is no longer supported for pnpm configuration [2]. - Configuration Setting: The setting for the package store directory is store-dir [3]. - Setting via CLI: You can also set this configuration persistently using the command: pnpm config set store-dir /path/to/.pnpm-store [3]. - Behavior: If no store-dir is configured, pnpm will automatically create a store on the same drive and filesystem where the installation is taking place [3][4]. For optimal performance and to enable hard linking, the store must reside on the same drive as the project [4][5]. If the store is on a different drive, pnpm will copy files instead of linking them [4][5].
Citations:
- 1: https://pnpm.io/configuring
- 2: https://pnpm.io/next/configuring
- 3: https://pnpm.io/10.x/configuring
- 4: https://pnpm.io/faq
- 5: https://github.com/pnpm/pnpm.io/blob/main/versioned_docs_archived/version-4.x/npmrc.md
Use pnpm_config_store_dir for the devcontainer store path
packageManager pins pnpm@11.8.0, which reads pnpm_config_*/PNPM_CONFIG_* only. npm_config_store_dir will be ignored, so the cache volume won’t take effect. Switch it to pnpm_config_store_dir: /pnpm-store.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.devcontainer/compose.devcontainer.yaml around lines 34 - 35, The
devcontainer environment is using the wrong store-path variable for pnpm, so the
cache volume will be ignored. In the devcontainer compose config, update the
environment entry under the devcontainer service from npm_config_store_dir to
pnpm_config_store_dir so pnpm@11.8.0 in the packageManager field can read it
correctly. Keep the value pointing to /pnpm-store and ensure the setting is
applied in the same environment block.
no ref The docker:up guard from the previous commit only covered docker:up's own command -- its dependsOn: [docker:build] still ran unconditionally, rebuilding ghost-dev-gateway's xcaddy-compiled Caddy image and ghost-dev's own image from inside the container on every Admin dev startup (Admin depends on docker:up, which depends on docker:build). That's real, wasted work in the devcontainer, where the images are already built/pulled by the devcontainer's own compose lifecycle before start-dev-stack.sh ever runs, and likely explains Admin's Vite still not coming up in testing after the docker:up fix alone. Applied the same CODESPACES/REMOTE_CONTAINERS guard.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
package.json (1)
129-129: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard logic correct; consider extracting shared devcontainer-detection logic.
The
docker:buildguard correctly mirrors thedocker:upguard at Line 114, and the$CODESPACES$REMOTE_CONTAINERSnon-empty check will correctly detect either a Codespaces or a VS Code Dev Containers environment (the extension setsREMOTE_CONTAINERS=trueinside the container). No functional issue here.Since this inline conditional is now duplicated across two nx targets, consider extracting it into a small shared script (e.g.
.devcontainer/skip-if-devcontainer.sh) to avoid repeating the same guard string if a third target needs it later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 129, The devcontainer guard string is duplicated between the `docker:build` and `docker:up` nx targets, so extract the shared `$CODESPACES$REMOTE_CONTAINERS` detection and skip behavior into a small reusable script or helper (for example a shared devcontainer guard script) and have both targets call it. Update the existing command strings in the package configuration to use that shared logic instead of embedding the same inline conditional twice, so any future target can reuse the same check consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@package.json`:
- Line 129: The devcontainer guard string is duplicated between the
`docker:build` and `docker:up` nx targets, so extract the shared
`$CODESPACES$REMOTE_CONTAINERS` detection and skip behavior into a small
reusable script or helper (for example a shared devcontainer guard script) and
have both targets call it. Update the existing command strings in the package
configuration to use that shared logic instead of embedding the same inline
conditional twice, so any future target can reuse the same check consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d0da9dd6-65a3-4caa-86fd-cbb2a951b5c7
📒 Files selected for processing (1)
package.json
no ref Ghost's session CSRF protection (cookieCsrfProtection in ghost/core/core/server/services/auth/session/session-service.js) compares the browser's Origin header against urlUtils.getAdminUrl(), which comes from Ghost's configured `url` setting -- defaulted to http://localhost:2368 (config.development.json). Codespaces serves everything through a forwarded https://<name>-2368.<domain> origin instead, so that check failed on every authenticated request and bounced the session back to login, in a loop -- not a restart wiping sessions (they're DB-backed via SessionStore, confirmed by reading express-session.js). url is a top-level nconf key, so a bare `url` env var overrides it. Set it dynamically from Codespaces' own CODESPACE_NAME + GITHUB_CODESPACES_PORT_FORWARDING_DOMAIN env vars, guarded so local (non-Codespaces) VS Code Dev Containers -- which really do forward to localhost, no mismatch -- are unaffected.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
no ref gh codespace ssh/logs need an SSH server in the container -- without one, diagnosing an issue means guessing from HTTP probes through gh codespace ports forward, which can't show CPU/memory pressure, load average, or which process is actually consuming resources. Adding ghcr.io/devcontainers/features/sshd:1 so a suspected VM resource problem can be checked directly instead of inferred.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
no ref SSH'd into a live devcontainer (added sshd feature in the previous commit specifically to get real data instead of guessing) and found two concrete resource issues via docker stats/ps aux under actual usage: - ember (ghost/admin's dev server, JOBS=4) was at 243% CPU / 2.5GB RSS continuously, well after its one-time build finished (frontend log confirmed a single 'Build successful', not a rebuild loop) -- i.e. idle watch mode itself was the cost. watchman wasn't installed in the image, so ember-cli's Watcher falls back to its much less efficient default Node-based watcher for a tree this large. Since VS Code Server's terminal/extension host run in the same container as ember, that sustained CPU hogging was directly stealing cycles from the terminal -- explaining the reported typing lag, not network latency. Installed watchman (confirmed available via apt on node:22.18.0-bullseye-slim: 4.9.0-5+b1); ember-cli auto-detects and prefers it over the Node watcher, no config needed. Only affects ghost-dev's own container -- the host-hybrid flow runs ember-cli on the developer's machine, not this image. - ghost-dev-mysql was at 439-459MiB of its 512MiB mem_limit (85-90%) in two separate live snapshots, not just at boot. That cap was set for a 2-core/8GB Codespace budget but applies unconditionally regardless of the selected machine tier. Raised to 1g (matching host-hybrid's unconstrained default) for real headroom above the actual ~450MiB working set; innodb-buffer-pool-size stays at 256M, so behavior on the minimum tier is unchanged, just less pinned.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
no ref The url override from the previous commit fixed the browser-facing CSRF/origin login loop, but broke apps/admin's own vite dev server: its bootstrap fetch (resolveGhostSiteUrl in apps/admin/vite-backend-proxy.ts) hits GHOST_URL + ghost/api/admin/site/ directly at http://localhost:2368, bypassing Caddy. Once Ghost's url is HTTPS, its url-redirects middleware (getAdminRedirectUrl) 301s any request it sees as insecure to that HTTPS url -- real browser traffic through Caddy is fine (Caddy sets X-Forwarded-Proto: https, so Express's req.secure is true), but vite's direct fetch has no such header, gets redirected to the public, GitHub-auth-gated tunnel URL, and crashes trying to parse the login page HTML as JSON. That crash cascades: Nx tears down the whole run-many continuous-task group when one task exits non-zero, taking Ember/Portal/shade/etc. down with it -- confirmed via SSH (added sshd specifically to get this kind of ground truth instead of guessing): a live snapshot caught ember as a defunct zombie following this exact crash trace in the frontend log. Fixed by routing GHOST_URL through the gateway container instead (http://ghost-dev-gateway:80/) -- confirmed via direct curl that this returns a clean 200 with the correct site JSON. Stays on the internal Docker network (no GitHub auth wall) while still picking up X-Forwarded-Proto: https from Caddy, so no redirect either.
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
1 participant
no ref
Summary
pnpm dev's default frontend fan-out to Admin + Portal only..devcontainer/start-dev-stack.sh, which previously hardcoded its own full frontend project list independently of rootpnpm dev, so it wasn't picking up the memory savings from Changed public app dev defaults #29105.nx run-manycommand, printed in the script's startup message.*_DEV_SERVERproxy targets for Portal/Comments UI/Signup Form/Sodo Search/Announcement Bar/Admin Toolbar fromdocker/dev-gateway/Dockerfile, the devcontainer's compose override, and forwarded ports indevcontainer.json. Unread since Removed public-app vite preview servers in favor of Caddy file_server #28970 switched public apps to Caddyfile_server; confirmed via grep the Caddyfile never references them. Fixing the shared Dockerfile removes the need to re-declare dead vars in the devcontainer overlay, and stops Codespaces showing five phantom forwarded ports nothing ever binds./ghost/indefinitely).apps/admin'sdevtargetdependsOn: ["ghost-monorepo:docker:up", ...](added in Changed dev target orchestration #29101 for the host flow, wherepnpm devis a single Nx invocation and dedupesdocker:upto one harmless run). The devcontainer'sstart-dev-stack.shinvokes Admin'sdevtarget via a separatenx run-manycall from the backend'spnpm --filter ghost dev, sodocker:upactually re-executes — and itsdocker compose ... up -d --force-recreate --wait, run from inside theghost-devcontainer via the mounted host docker socket, recreates the very container issuing the command. Portal has no such dependency, which is why it always loaded fine while Admin hung. Guarded the recreate behindCODESPACES/REMOTE_CONTAINERS(set inside the container by both Codespaces and VS Code Dev Containers) — redundant there anyway since the devcontainer's own compose lifecycle already brings services up beforestart-dev-stack.shruns.Test plan
bash -npasses on the modified scriptdocker compose -f compose.dev.yaml -f .devcontainer/compose.devcontainer.yaml configmerges cleanly with only the four live gateway env varsnode -e "JSON.parse(...)"onpackage.jsonafter thedocker:upguard; verified the shell conditional takes the correct branch with/withoutCODESPACESset/ghost/(Admin) loads instead of 502ing indefinitely