Skip to content

[containers-shared] Fix Docker spawn opening console windows on Windows#13017

Open
petebacondarwin wants to merge 1 commit intomainfrom
fix/windows-docker-console-windows
Open

[containers-shared] Fix Docker spawn opening console windows on Windows#13017
petebacondarwin wants to merge 1 commit intomainfrom
fix/windows-docker-console-windows

Conversation

@petebacondarwin
Copy link
Contributor

@petebacondarwin petebacondarwin commented Mar 23, 2026

Fixes #12963.

When running wrangler deploy on Windows with [[containers]] configured, the Docker build step spawns numerous console windows that flash open during the build process.

This is caused by detached: true in child_process.spawn() calls — on Windows, detached: true gives each child process its own visible console window (Node.js docs).

The fix:

  • Sets detached conditionally: true on Unix/macOS (needed for process group cleanup via process.kill(-pid)), false on Windows
  • Adds windowsHide: true to further suppress console windows on Windows (no-op on other platforms)
  • Guards the process.kill(-child.pid) abort logic with a platform check: uses child.kill() on Windows since negative-PID process group kill is a Unix-only concept

Both dockerBuild() in build.ts and runDockerCmd() in utils.ts are updated.


  • Tests
    • Tests included/updated
    • Automated tests not possible - manual testing has been completed as follows:
    • Additional testing not necessary because: This is a platform-specific behavior change (Windows detached and windowsHide options). Existing tests pass. The behavior difference is only observable on Windows with Docker Desktop, which cannot be tested in CI. The logic change is minimal and well-documented in Node.js docs.
  • Public documentation
    • Cloudflare docs PR(s):
    • Documentation not necessary because: This is a bug fix with no user-facing API changes.

Open with Devin

@petebacondarwin petebacondarwin requested review from a team as code owners March 23, 2026 14:36
@changeset-bot
Copy link

changeset-bot bot commented Mar 23, 2026

🦋 Changeset detected

Latest commit: 777809a

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@workers-devprod
Copy link
Contributor

workers-devprod commented Mar 23, 2026

Codeowners approval required for this PR:

  • @cloudflare/wrangler
  • ✅ @cloudflare/cloudchamber
Show detailed file reviewers
  • packages/containers-shared/src/build.ts: [@cloudflare/wrangler]
  • packages/containers-shared/src/utils.ts: [@cloudflare/wrangler]

@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2026

✅ All changesets look good

@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 23, 2026

I've reviewed the full diff and both affected files. Let me now analyze the changes:

  1. detached: process.platform !== "win32" - Correct. On Windows, detached: true creates new console windows. On Unix, it's needed for process group management.

  2. windowsHide: true - Correct. Belt-and-suspenders approach, no-op on non-Windows.

  3. Abort logic platform guard - On Windows, falls back to child.kill() since process.kill(-pid) (negative PID / process group kill) is a Unix-only concept. This is correct.

  4. Changeset targets "wrangler": patch — the changed package is @cloudflare/containers-shared which is private and not published independently. Since containers-shared is bundled into wrangler, attributing the changeset to wrangler is correct.

  5. Code duplication — The spawn options and abort logic are now identically duplicated between build.ts and utils.ts. This is a pre-existing concern and not introduced by this PR (the duplication existed before), so not actionable here.

  6. Windows abort only kills the direct child — On Windows with detached: false, child.kill() only terminates the direct child process, not its descendants. Docker's child processes (buildkit, etc.) could potentially be left behind. However, this is a pragmatic trade-off: the previous behavior on Windows was worse (flashing console windows), and Docker Desktop on Windows generally handles its own subprocess cleanup. This is not a regression since detached: true + process.kill(-pid) on Windows would have been unreliable anyway (the Node.js docs state the behavior differs).

The logic is sound, the platform checks are correct, and the comments are accurate.

LGTM

github run

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 23, 2026

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13017

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13017

miniflare

npm i https://pkg.pr.new/miniflare@13017

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13017

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13017

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13017

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13017

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13017

wrangler

npm i https://pkg.pr.new/wrangler@13017

commit: 777809a

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Mar 23, 2026
On Windows, `detached: true` in child_process.spawn() gives each child
process its own visible console window. This caused many windows to flash
open during `wrangler deploy` with `[[containers]]` configured.

- Set `detached` conditionally: true on Unix (for process group cleanup),
  false on Windows (to prevent console windows)
- Add `windowsHide: true` to further suppress console windows on Windows
- Guard `process.kill(-pid)` abort logic: use `child.kill()` on Windows
  since negative-PID process group kill is Unix-only

Fixes #12963
@petebacondarwin petebacondarwin force-pushed the fix/windows-docker-console-windows branch from ad73c62 to 777809a Compare March 23, 2026 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Approved

Development

Successfully merging this pull request may close these issues.

Windows: Docker container builds spawn many console windows

3 participants