Skip to content

Moved dev stack auto-start from VS Code tasks to postAttachCommand#27547

Merged
9larsons merged 7 commits intomainfrom
fix-devcontainer-autostart
Apr 24, 2026
Merged

Moved dev stack auto-start from VS Code tasks to postAttachCommand#27547
9larsons merged 7 commits intomainfrom
fix-devcontainer-autostart

Conversation

@9larsons
Copy link
Copy Markdown
Contributor

@9larsons 9larsons commented Apr 24, 2026

Summary

Follow-up to #27544 and #27545. The auto-start flow shipped in #27545 used a runOn: "folderOpen" task in .vscode/tasks.json, which fires on host VS Code too — before the user clicks "Reopen in Container". On a fresh clone the host has no node_modules, so the task crashes with sh: nodemon: command not found + Command "nx" not found, greeting first-time contributors with a wall of red errors.

This PR moves auto-start into the devcontainer lifecycle so it only fires inside the container, and also fixes a second fresh-clone failure mode discovered while testing: Ghost's backend crashes importing @tryghost/parse-email-address because its build/ output doesn't exist yet.

Changes

  • .devcontainer/start-dev-stack.sh (new) — backgrounds pnpm --filter ghost dev and the Nx run-many -t dev for all six frontend apps via nohup + disown. Output goes to /tmp/ghost-backend.log and /tmp/ghost-frontends.log. A /dev/tcp probe on port 2368 makes the script idempotent, so VS Code reloads don't double-start processes.
  • .devcontainer/devcontainer.json — adds postAttachCommand pointing at the new script. postAttachCommand is a Dev Containers lifecycle hook that only fires inside the container, eliminating the host-side fire-and-fail.
  • .devcontainer/postCreate.sh — adds pnpm build after pnpm install. On host, pnpm dev goes through nx run ghost-monorepo:docker:dev whose dependsOn cascades build internal @tryghost/* workspace packages before starting watchers. In the devcontainer we invoke pnpm --filter ghost dev directly, which bypasses those cascades, so the backend crashes on a fresh clone trying to require('@tryghost/parse-email-address/build/index.js'). Running pnpm build once during postCreate fills in those build outputs; Nx caches subsequent invocations.
  • .vscode/tasks.json — removes runOptions.runOn: "folderOpen" from the Ghost: Full dev stack task (original Moved dev stack auto-start from VS Code tasks to postAttachCommand #27547 scope). The task is still available via ⇧⌘B, but it no longer auto-fires on folder open.

Trade-off

Dev logs are in /tmp/ghost-{backend,frontends}.log instead of VS Code task panels. The upside is zero-interaction startup inside the container and no way to trigger failures on host. ⇧⌘B is still available for anyone who prefers the task-panel experience.

End-to-end verification

Tested locally with a fresh clone into a second workspace (~/dev/ghost-dc/Ghost):

  1. git clone + code . on host → no tasks auto-fire, no red errors. Only "Reopen in Container" toast appears.
  2. Reopen in Container → postCreate runs pnpm install + pnpm build (one-time ~5–8 min).
  3. VS Code attaches → postAttachCommand fires → script backgrounds the stack.
  4. Wait ~30–60s, http://localhost:2368/ghost/ loads admin. Smoke probes from the gateway:
    200  /
    200  /ghost/
    200  /ghost/api/admin/site/
    200  /ghost/assets/portal/portal.min.js
    
  5. Close VS Code → shutdownAction: stopCompose stops all five services within ~10s (exit code 137 after the grace period; Ghost's nodemon/pnpm stack doesn't handle SIGTERM cleanly, so they take the full 10s). Containers remain as Exited in docker ps -a, ready for fast restart on next Reopen.

Why one PR and not two

The original #27547 removed runOn: folderOpen but regressed auto-start inside the container too. Shipping just that change would leave contributors with a devcontainer that attaches silently and requires them to hunt for the right command. The new postAttachCommand + pre-build steps are the complete answer: they restore auto-start without the host-side failure mode. One commit replaces another on the same branch — reviewable as a single story.

Test plan

  • Fresh host clone + code . — no tasks auto-fire, no red errors on host Terminal panel.
  • Reopen in Container — postCreate completes successfully (pnpm install + pnpm build).
  • postAttachCommand visibly fires after attach (check "Dev Containers" output channel).
  • http://localhost:2368/ghost/ and http://localhost:2368/ both return 200 within 60s of attach.
  • Close VS Code window — within 10s containers move to Exited in docker ps -a.
  • Reopen Ghost in container → stack auto-restarts; idempotency guard skips if something's still listening on 2368.
  • Codespaces on this branch: create a codespace → cold start completes → Ghost reachable via the forwarded 2368 URL without manual commands.

- runOn: folderOpen fires when VS Code opens the workspace on host too, not just inside the devcontainer
- on a fresh host clone there's no node_modules yet, so the auto-fired tasks crash with "nodemon: command not found" and "Command nx not found" before the user has even seen the Reopen in Container prompt
- keep group.isDefault so Shift+Cmd+B still triggers Ghost: Full dev stack after the user chooses to start it manually
- a container-only auto-start (via devcontainer.json postAttachCommand) is a better path than gating the task on an env check; tracked as a follow-up
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 24, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Removes runOptions.runOn: "folderOpen" from the VS Code "Ghost: Full dev stack" task. Adds features and a postAttachCommand to .devcontainer/devcontainer.json to run .devcontainer/start-dev-stack.sh on editor attach. Updates .devcontainer/postCreate.sh to build @tryghost/parse-email-address. Adds .devcontainer/start-dev-stack.sh and extends .devcontainer/compose.devcontainer.yaml with mysql, redis, and mailpit services.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: moving dev stack auto-start from VS Code tasks to the devcontainer postAttachCommand lifecycle hook.
Description check ✅ Passed The description is comprehensive and directly related to the changeset, explaining the rationale, changes made, trade-offs, and verification steps.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-devcontainer-autostart

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

The first revision of this PR simply dropped runOn: folderOpen from
tasks.json because the task fired on host VS Code before the user had
a chance to Reopen in Container, failing loudly with "nodemon not found"
on a fresh clone with no node_modules. That avoided the scary first-run
UX but regressed auto-start inside the container too.

This revision restores auto-start, but scopes it to the devcontainer
lifecycle so the host is never touched:

- .devcontainer/start-dev-stack.sh backgrounds the backend nodemon and the
  six Vite dev servers via nohup + disown. Logs go to /tmp/ghost-backend.log
  and /tmp/ghost-frontends.log. A /dev/tcp guard on port 2368 makes the
  script idempotent, so VS Code reloads don't double-start processes.
- devcontainer.json adds postAttachCommand pointing at the script, so the
  stack starts on every attach but only inside the container (postAttach is
  a devcontainer lifecycle hook, not a VS Code task).
- postCreate.sh adds `pnpm build` after `pnpm install`. On host, `pnpm dev`
  goes through `nx run ghost-monorepo:docker:dev` whose dependsOn cascades
  build internal @tryghost/* workspace packages before starting watchers.
  In the devcontainer we invoke `pnpm --filter ghost dev` directly, which
  bypasses those cascades, so the backend crashes on a fresh clone trying
  to require @tryghost/parse-email-address/build/index.js. Pre-building
  via pnpm build fills in those build outputs once; Nx caches subsequent
  invocations, so rebuilds are cheap.

Trade-off vs the task-panel approach: dev logs are in /tmp instead of VS
Code task panels, but the upside is zero-interaction startup inside the
container and no way to trigger failures on host.
@9larsons 9larsons changed the title Removed folder-open auto-run from Ghost: Full dev stack task Moved dev stack auto-start from VS Code tasks to postAttachCommand Apr 24, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.devcontainer/start-dev-stack.sh:
- Around line 9-22: Update the startup guard to check each service before
(re)starting instead of only probing 127.0.0.1:2368: probe the backend (used by
pnpm --filter ghost dev) on :2368 and probe each frontend Vite port (5174, 4175,
7173, 6174, 4178, 4177 used by pnpm nx run-many -t dev) and only start the
missing ones; change the log redirection for /tmp/ghost-backend.log and
/tmp/ghost-frontends.log from truncation (>) to append (>>) and prepend a
timestamped banner when starting a service so previous crash tails are
preserved. Ensure you use the existing process-start commands (pnpm --filter
ghost dev and pnpm nx run-many -t dev) and only invoke them for services whose
port probe failed to avoid EADDRINUSE races.
🪄 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: eac84af8-d5a2-4248-8021-a6e883b8cccc

📥 Commits

Reviewing files that changed from the base of the PR and between c45d484 and 9e98845.

📒 Files selected for processing (3)
  • .devcontainer/devcontainer.json
  • .devcontainer/postCreate.sh
  • .devcontainer/start-dev-stack.sh

Comment thread .devcontainer/start-dev-stack.sh
Running `pnpm build` at the monorepo root triggers `nx run-many -t build`
across every workspace package — 16+ parallel builds, ~10 minutes on a
2-core Codespace, and sensitive to unrelated build failures (e.g. a
broken @tryghost/stats:build taking down the whole postCreate via
set -euo pipefail).

But only one workspace package actually needs pre-building for the
backend to start: @tryghost/parse-email-address, whose package.json main
points at build/index.js so ghost/core can't `require` it without the
compiled output. @tryghost/i18n is plain JS. Everything else is either
a frontend app with its own dev target (admin, posts, stats,
activitypub, portal, comments-ui, signup-form, sodo-search,
announcement-bar) or a library that ghost/core doesn't import with a
build output.

Narrowing to just parse-email-address cuts postCreate from ~10 min to
<1 min on a fresh Codespace and removes the cross-project failure
coupling. If new workspace packages with build outputs are introduced
that ghost/core imports, add them to the filter list here.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
.devcontainer/postCreate.sh (1)

12-23: LGTM — targeted pre-build is a sensible trade-off.

Narrowing the postCreate build to @tryghost/parse-email-address addresses the fresh-clone import failure without pulling in the full monorepo build, which keeps Codespaces cold-start fast. Rationale is well documented inline.

One small maintainability note (non-blocking): the comment correctly calls out that this is the only workspace package today whose main points at a build/ artifact consumed by ghost/core at runtime when bypassing Nx dependsOn cascades. If another such package is introduced later, this will silently regress on fresh clones until someone remembers to extend this list. Consider, as a follow-up, either driving this from an Nx target (e.g. nx run parse-email-address:build, or a small meta-target like precontainer-build that packages opt into) so new packages are picked up automatically rather than by convention.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/postCreate.sh around lines 12 - 23, Replace the hardcoded pnpm
filter call that builds `@tryghost/parse-email-address` by invoking the Nx build
target (or a new meta-target) so future workspace packages opt into the
precontainer build automatically; specifically, change the postCreate step that
currently runs "pnpm --filter `@tryghost/parse-email-address` build" to call "nx
run parse-email-address:build" or create and call a shared "precontainer-build"
target that packages can declare, updating the postCreate script to run that Nx
target instead of the explicit pnpm filter.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In @.devcontainer/postCreate.sh:
- Around line 12-23: Replace the hardcoded pnpm filter call that builds
`@tryghost/parse-email-address` by invoking the Nx build target (or a new
meta-target) so future workspace packages opt into the precontainer build
automatically; specifically, change the postCreate step that currently runs
"pnpm --filter `@tryghost/parse-email-address` build" to call "nx run
parse-email-address:build" or create and call a shared "precontainer-build"
target that packages can declare, updating the postCreate script to run that Nx
target instead of the explicit pnpm filter.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f1bf7dae-5de2-4446-9c48-05159b801ddc

📥 Commits

Reviewing files that changed from the base of the PR and between 9e98845 and c8209b1.

📒 Files selected for processing (1)
  • .devcontainer/postCreate.sh

Developers attached to the ghost-dev container can't diagnose peer compose
services (mysql, redis, mailpit, gateway) without docker CLI access — the
alternative is SSHing to the Codespaces host, which our image doesn't
support out of the box, or rebuilding the Codespace from scratch every
time something misbehaves.

Adding the Microsoft-maintained docker-outside-of-docker feature mounts the
host docker socket and installs docker CLI inside ghost-dev, so commands
like `docker ps`, `docker logs ghost-dev-mysql`, `docker inspect` and
`docker restart` work from the attached terminal.

This is a common pattern for compose-based devcontainers; it is not a
privilege escalation of the dev container (the socket is read-write but
already a standard devcontainer feature maintained by Microsoft). Adds
~50 MB to the image for the docker CLI itself.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
.devcontainer/devcontainer.json (1)

18-20: Feature version pin is loose (:1) — fine for dev, just flagging.

ghcr.io/devcontainers/features/docker-outside-of-docker:1 tracks the latest 1.x release, so a new minor/patch can change behavior between rebuilds. For a dev container that's the convention and rebuilds are cheap, so this is acceptable. If you want fully reproducible dev environments, pin to the exact version (e.g., :1.0.0) or use the SHA digest.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/devcontainer.json around lines 18 - 20, The feature reference
"ghcr.io/devcontainers/features/docker-outside-of-docker:1" pins only to the 1.x
channel which allows minor/patch updates to change behavior; update that feature
string in .devcontainer/devcontainer.json to a specific immutable identifier
(either an exact semantic version like :1.0.0 or the image SHA digest) so the
devcontainer rebuilds are fully reproducible and will not pull unintended
updates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.devcontainer/devcontainer.json:
- Line 33: postAttachCommand currently runs bash
.devcontainer/start-dev-stack.sh which uses a /dev/tcp/127.0.0.1/2368 probe to
avoid double-starts, but that probe can miss race windows and doesn't protect
frontend restarts; update start-dev-stack.sh to acquire a durable lock (e.g.,
use flock on a PID/lock file such as /tmp/ghost-dev-stack.lock) around the
startup sequence so only one concurrent invoker runs pnpm --filter ghost dev and
nx run-many -t dev, and ensure the lock is released on exit so reattaches can
restart crashed frontends.
- Around line 12-20: Add a short explanatory comment above the "features" block
documenting the security trade-off of using
"ghcr.io/devcontainers/features/docker-outside-of-docker:1" together with
"remoteUser": "root": state that this feature mounts the host Docker socket into
the container and installs the Docker CLI, which effectively grants
root-equivalent control over the host Docker daemon (able to start privileged
containers and mount host paths), and warn contributors to avoid this pattern in
untrusted/shared/CI environments or to use alternatives (DinD, remote Docker
hosts, or restricted user mappings) where appropriate.

---

Nitpick comments:
In @.devcontainer/devcontainer.json:
- Around line 18-20: The feature reference
"ghcr.io/devcontainers/features/docker-outside-of-docker:1" pins only to the 1.x
channel which allows minor/patch updates to change behavior; update that feature
string in .devcontainer/devcontainer.json to a specific immutable identifier
(either an exact semantic version like :1.0.0 or the image SHA digest) so the
devcontainer rebuilds are fully reproducible and will not pull unintended
updates.
🪄 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: e7101971-dbb3-415b-b79f-9672b48fe2e7

📥 Commits

Reviewing files that changed from the base of the PR and between c8209b1 and 792b6fd.

📒 Files selected for processing (1)
  • .devcontainer/devcontainer.json

Comment thread .devcontainer/devcontainer.json
// container, so this is safe (unlike a tasks.json runOn: folderOpen which
// also fires on host VS Code where node_modules doesn't exist yet).
// The script guards against double-starting if the stack is already up.
"postAttachCommand": "bash .devcontainer/start-dev-stack.sh",
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

postAttachCommand re-runs on every attach — confirm the idempotency guard is sufficient.

postAttachCommand fires on every VS Code attach/reattach, not just the first one. The guard in start-dev-stack.sh (the /dev/tcp/127.0.0.1/2368 probe) handles the common case, but note two edge cases:

  1. Between the moment the backend process is launched and when it binds port 2368 (can be several seconds on a cold start), a second attach would see the port closed and launch a second pnpm --filter ghost dev. The first-run window is small, but a user opening two VS Code windows against the same container within that window would hit it.
  2. The frontends (nx run-many -t dev) aren't port-guarded by the 2368 probe — they're gated behind the same early-exit, which is fine, but it means if the backend is up and frontends crashed, a reattach won't restart them.

Neither is a blocker; consider a PID/lock-file guard (e.g., flock on /tmp/ghost-dev-stack.lock) if you want to harden this later.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/devcontainer.json at line 33, postAttachCommand currently runs
bash .devcontainer/start-dev-stack.sh which uses a /dev/tcp/127.0.0.1/2368 probe
to avoid double-starts, but that probe can miss race windows and doesn't protect
frontend restarts; update start-dev-stack.sh to acquire a durable lock (e.g.,
use flock on a PID/lock file such as /tmp/ghost-dev-stack.lock) around the
startup sequence so only one concurrent invoker runs pnpm --filter ghost dev and
nx run-many -t dev, and ensure the lock is released on exit so reattaches can
restart crashed frontends.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. The race windows you describe are real but small in practice:

  1. Between nohup pnpm --filter ghost dev & and port 2368 binding, a second attach inside the same container could spawn a second backend. In normal VS Code usage reattaches are not concurrent (a new window attaches only after the first has fully re-opened), so this would require the user opening two VS Code windows against the same Codespace within the same ~30s boot window.
  2. Backend-alive-but-frontend-dead → reattach short-circuits. Real.

Rather than reach for flock now, we're leaving the idempotency guard simple and documenting the edge cases in DEVCONTAINER-FOLLOWUPS.md. A lockfile (or per-target port probes) is a reasonable hardening step if/when these scenarios come up in real usage. The other CodeRabbit comment on start-dev-stack.sh:22 tracks the same issue.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@9larsons — Sounds reasonable. The concurrent-attach race window is genuinely narrow in normal VS Code/Codespaces usage, and deferring flock until there's a real-world trigger is a sensible call. Good to know the follow-up is captured in DEVCONTAINER-FOLLOWUPS.md; flagging it there (and in the linked comment on start-dev-stack.sh:22) should be enough to pick it up when the time is right.

Baseline compose.dev.yaml tunes MySQL for a beefy workstation:

  --innodb-buffer-pool-size=1G --innodb-log-buffer-size=500M

1.5 GB resident in MySQL before Ghost, 6 Vite dev servers, nodemon, and
Docker overhead all take their share — fine on an 8-core / 32 GB host, but
on a 2-core / 8 GB Codespace MySQL gets pushed around enough that
something upstream eventually shuts it down clean (SIGTERM, exit 0) ~5
min in. Ghost's knex pool then fails DNS on reconnect and nodemon
crashes.

The overlay pins a dev-sized profile:

  - innodb buffer pool 256M, log buffer 64M — still ample for the fixture
    data volumes Ghost generates in dev, cuts MySQL's resident set by ~1.2 GB
  - mem_limit 512m — backstop so MySQL can't quietly grow into its old size
  - restart: unless-stopped on mysql, redis, mailpit — if something does
    knock one of them over, compose brings it back and Ghost's next nodemon
    cycle reconnects, rather than the dev stack staying wedged until manual
    rescue

Only affects the devcontainer path; the original compose.dev.yaml values
are untouched for host users of `pnpm dev`.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.devcontainer/compose.devcontainer.yaml:
- Around line 7-8: The cgroup memory limit (mem_limit) in the devcontainer
compose file is likely too low given the MySQL flags
(--innodb-buffer-pool-size=256M and --innodb-log-buffer-size=64M) and MySQL 8.x
baseline usage; update mem_limit to 768m or 1g to avoid OOM-killing mysqld, or
alternatively reduce the --innodb-log-buffer-size to a lower value (e.g., 16M)
to keep total resident memory under the current 512m; edit the entries for
mem_limit and/or the command string (containing --innodb-buffer-pool-size and
--innodb-log-buffer-size) to implement the chosen change.
- Line 7: The devcontainer's command override dropped the
--innodb-change-buffer-max-size=50 flag from the base MySQL command; update the
command entry that sets --innodb-buffer-pool-size, --innodb-log-buffer-size,
--innodb-flush-log-at-trx_commit and --innodb-flush-method to also include
--innodb-change-buffer-max-size=50 (or, if intentional, add a brief inline
comment next to the command explaining why the flag was removed) so the
devcontainer either preserves parity with the base profile or documents the
deliberate omission.
🪄 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: 0e6a7c44-6612-4adb-b77f-656f245eff26

📥 Commits

Reviewing files that changed from the base of the PR and between 792b6fd and 7fc9fad.

📒 Files selected for processing (1)
  • .devcontainer/compose.devcontainer.yaml

Comment on lines +7 to +8
command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=64M --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
mem_limit: 512m
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

mem_limit: 512m may be too tight and trigger OOM-kills.

Budget check: InnoDB buffer pool (256M) + log buffer (64M) already accounts for ~320 MiB before MySQL 8.4's own baseline (dictionary cache, per-connection buffers, thread stacks, performance_schema, etc.), which typically sits in the 250–400 MiB range even idle. Under the first schema migration or a burst of connections from the dev stack, the 512 MiB cgroup limit can be exceeded and the kernel will OOM-kill mysqld — which will appear as a flaky dev loop rather than a clear error.

Consider raising to ~768m–1g, or lowering innodb-log-buffer-size much closer to the 16M default (the baseline's 500M was unusual; 64M is still generous for dev writes).

🔧 Suggested adjustment
-    command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=64M --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
-    mem_limit: 512m
+    command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=16M --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
+    mem_limit: 768m
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/compose.devcontainer.yaml around lines 7 - 8, The cgroup
memory limit (mem_limit) in the devcontainer compose file is likely too low
given the MySQL flags (--innodb-buffer-pool-size=256M and
--innodb-log-buffer-size=64M) and MySQL 8.x baseline usage; update mem_limit to
768m or 1g to avoid OOM-killing mysqld, or alternatively reduce the
--innodb-log-buffer-size to a lower value (e.g., 16M) to keep total resident
memory under the current 512m; edit the entries for mem_limit and/or the command
string (containing --innodb-buffer-pool-size and --innodb-log-buffer-size) to
implement the chosen change.

# Lets the dev stack stay within a 2-core / 8 GB Codespace's budget
# after Ghost + 6 Vite dev servers are running. 256 MB is plenty for
# dev data volumes.
command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=64M --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

find . -type f -name "compose*.yaml" -o -name "compose*.yml" | head -20

Repository: TryGhost/Ghost

Length of output: 228


🏁 Script executed:

cd .devcontainer && pwd && ls -la | grep -E "compose|yaml"

Repository: TryGhost/Ghost

Length of output: 167


🏁 Script executed:

cat -n compose.dev.yaml 2>/dev/null || cat -n compose.devcontainer.yaml 2>/dev/null || echo "Files not found in this directory"

Repository: TryGhost/Ghost

Length of output: 5877


🏁 Script executed:

cat -n .devcontainer/compose.devcontainer.yaml

Repository: TryGhost/Ghost

Length of output: 2473


command overrides completely — --innodb-change-buffer-max-size=50 is omitted without explanation.

The devcontainer overrides the MySQL command line without including --innodb-change-buffer-max-size=50 from the baseline. While the comments explain the memory reduction goal (buffer pool 1G → 256M), they don't address why this flag was dropped. Since innodb-change-buffer-max-size=50 has no meaningful impact on RSS, it should either be restored to maintain parity with the base profile, or the omission should be explicitly documented if intentional.

🔧 Re-add the dropped flag if unintentional
-    command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=64M --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
+    command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=64M --innodb-change-buffer-max-size=50 --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=64M --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
command: --innodb-buffer-pool-size=256M --innodb-log-buffer-size=64M --innodb-change-buffer-max-size=50 --innodb-flush-log-at-trx_commit=0 --innodb-flush-method=O_DIRECT
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.devcontainer/compose.devcontainer.yaml at line 7, The devcontainer's
command override dropped the --innodb-change-buffer-max-size=50 flag from the
base MySQL command; update the command entry that sets
--innodb-buffer-pool-size, --innodb-log-buffer-size,
--innodb-flush-log-at-trx_commit and --innodb-flush-method to also include
--innodb-change-buffer-max-size=50 (or, if intentional, add a brief inline
comment next to the command explaining why the flag was removed) so the
devcontainer either preserves parity with the base profile or documents the
deliberate omission.

…json

- start-dev-stack.sh now appends to /tmp/ghost-backend.log and
  /tmp/ghost-frontends.log instead of truncating, with a timestamped
  banner on each start. Previous crash tails survive restarts so users
  can still read what went wrong without race-catching the log before
  nodemon restarts
- devcontainer.json gains an explicit security note on the
  docker-outside-of-docker + remoteUser: root combination. Standard
  pattern for local/Codespaces dev containers but worth flagging so the
  pattern isn't replicated into shared build agents / CI runners /
  untrusted-code contexts

Deferred: per-target port guard (or flock lock) in start-dev-stack.sh
to fully handle partial-failure restarts. Tracked in
DEVCONTAINER-FOLLOWUPS.md.
PR #27545 added `.devcontainer/**` and `.vscode/**` to the `any-code`
path-filter in ci.yml so PRs touching only those directories skip the
most expensive CI jobs. But Nx's `--affected` computation has a separate
model: when a changed file isn't inside any project and isn't listed in
`sharedGlobals` in nx.json, Nx defaults to "unknown → everything
affected" as a safety measure. That makes `pnpm nx show projects
--affected` return the whole workspace, which makes `affected_projects_str`
non-empty in job_setup, which in turn unskips Lint, Unit tests, and the
per-app `build_packages` matrix.

`.nxignore` uses .gitignore syntax and tells Nx to drop matching paths
from the project graph entirely. Changes confined to .devcontainer/ or
.vscode/ now produce an empty affected set — Lint / Unit / Build skip
cleanly — without needing conditional logic sprinkled across every
downstream job.

This PR itself still runs the full suite because we're touching
.github/workflows/ci.yml config, which always retriggers the CI
pipeline by design.
@sonarqubecloud
Copy link
Copy Markdown

@9larsons 9larsons enabled auto-merge (squash) April 24, 2026 20:02
@9larsons 9larsons merged commit 8306eb7 into main Apr 24, 2026
42 checks passed
@9larsons 9larsons deleted the fix-devcontainer-autostart branch April 24, 2026 20:29
9larsons added a commit that referenced this pull request Apr 24, 2026
ref https://linear.app/ghost/issue/PLA-37/

Stacks on **#27547** (pending merge). First piece of the Codespaces
cold-start optimisation work from `DEVCONTAINER-FOLLOWUPS.md` (F1).

Adds `.github/workflows/devcontainer-build.yml` — a GHA workflow that
builds `docker/ghost-dev/Dockerfile` and publishes the result to
`ghcr.io/tryghost/ghost-devcontainer:{latest,<sha>}` on merges to
`main`. The published image is the prerequisite for switching
`.devcontainer/devcontainer.json` to an `image:` reference, which would
drop a fresh Codespace's cold start from the current ~5–10 min (image
build + pnpm install) to ~30–60 s (image pull).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant