fix: keep run --local working on Windows (process liveness + tree teardown)

.github/workflows/push-test.yml

@@ -51,6 +51,42 @@ jobs:
             exit 1
           fi
 
+  # The Windows process helpers (cmd/mxcli/docker/procgroup_windows.go) cannot be
+  # exercised by the ubuntu job: Signal(0) succeeds on Linux and there is no
+  # process tree to reap. `mxcli run --local` hung forever on Windows at
+  # "Starting mxbuild --serve..." because alive() asked Signal(0) (unsupported on
+  # Windows, so a live mxbuild read as dead) and Stop() killed only the wrapper,
+  # leaving mxbuild's Deno web-ext worker holding the stdout pipe so cmd.Wait()
+  # blocked. This job runs the Windows-only regression tests on a real Windows
+  # runner so neither half can come back.
+  windows-process-regression:
+    runs-on: windows-latest
+    steps:
+      - uses: actions/checkout@v7
+      - uses: actions/setup-go@v7
+        with:
+          go-version: '1.26.6'
+      - name: Test the Windows process helpers
+        shell: bash
+        # Scoped with -run: the rest of the docker package's tests assert POSIX
+        # file modes and shell out to `sh`, so the full package does not pass on
+        # Windows (tracked in #897). The test binary is still COMPILED for this
+        # platform, so a Windows build break in the package is caught too.
+        #
+        # -run can pass vacuously if the tests are renamed or deleted, so assert
+        # that the expected number actually ran.
+        run: |
+          out=$(go test -v -count=1 -run 'TestProcessAlive|TestKillProcessGroup|TestServeServer_AliveTracksProcess|TestLocalRuntime_AliveTracksProcess' ./cmd/mxcli/docker/)
+          echo "$out"
+          n=$(printf '%s\n' "$out" | grep -c '^--- PASS: Test' || true)
+          echo "windows process tests executed: $n"
+          if [ "$n" -lt 5 ]; then
+            echo "FAIL: expected at least 5 Windows process tests to run, -run matched $n."
+            echo "      procgroup_windows_test.go must keep its processAlive / tree-kill /"
+            echo "      alive() tests, or the Windows local-boot regression is unguarded."
+            exit 1
+          fi
+
   build-and-test:
     runs-on: ubuntu-latest
     steps:

CHANGELOG.md

@@ -38,6 +38,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ### Fixed
 
+- **`mxcli run --local` hung forever on Windows at `Starting mxbuild --serve...`** — two POSIX assumptions in `cmd/mxcli/docker` combined. `ServeServer.alive()` (and `LocalRuntime.alive()`) asked `os.Process.Signal(syscall.Signal(0))`, which Go implements as `EWINDOWS` ("not supported by windows") for every signal but `Kill` — so a live mxbuild read as dead and `waitReady()` aborted the boot the instant it launched it. And `Stop()` waited on `cmd.Wait()` after a `killProcessGroup` that on Windows only called `p.Kill()`: mxbuild.exe is a wrapper that launches a Deno web-ext worker (`modeler/tools/deno/win-x64/deno.exe`) which inherits the stdout/stderr pipe `exec.Cmd` hands out, so the orphan kept the pipe open and `Wait()` never saw EOF — no error, no exit, just a hang. Windows now has a real liveness check (`OpenProcess(SYNCHRONIZE)` + `WaitForSingleObject`) and a tree kill (`taskkill /F /T`) behind `signalProcessGroup`/`killProcessGroup`; the POSIX implementations are unchanged. The Windows-only tests in `procgroup_windows_test.go` pin both halves — the tree-kill test fails on the pre-fix code, verified as a control — and a `windows-latest` CI job runs them (asserting they actually executed) so neither can come back.
+
 - **`raise error;` on a microflow's main flow passed check and exec, then failed the build** (mendixlabs/mxcli#1030) — with `[error] [CE0710] "The main flow cannot join an error flow or end in an error event."`, one per microflow. Mendix's error event *re-raises the error being handled*, so it is legal only where an error is in scope: inside an `on error { … }` handler. Studio Pro will not draw the connection from the normal flow to an error event; mxcli could, and did. It is now **MDL084**, at error severity, so `exec`'s pre-flight refuses the script with nothing written (`--no-check` still applies it, for reproducing the build failure).
 
   The report's own diagnosis — a trailing End event appended because `RaiseErrorStmt` never set the builder's "ends with return" flag — is not the cause: `isTerminalStmt` has treated it as a terminator all along, and the graph mxcli builds for `raise error;` is exactly the one the report asks for (start event, error event, one sequence flow, no trailing End event, no outgoing flow), pinned by a test. No wiring makes a main-flow error event legal, which is why the fix is a refusal rather than a builder change.

cmd/mxcli/docker/localboot.go

@@ -711,16 +711,18 @@ func (rt *LocalRuntime) Log() string { return rt.log.String() }
 
 // alive reports whether the runtime process is still running.
 //
-// Signal(0) is only a correct liveness test BECAUSE watchExit reaps: it succeeds
-// on an unreaped zombie — measured, proc state `Z`, err nil — and returns
-// "process already finished" once Wait has run. Before the reaper existed this
-// function reported a runtime that had terminated itself hours earlier as alive
-// (mxcli-formula1 FINDINGS §60). Removing watchExit silently breaks this line.
+// Signal(0) is only a correct liveness test on POSIX, and even there only
+// BECAUSE watchExit reaps: it succeeds on an unreaped zombie — measured, proc
+// state `Z`, err nil — and returns "process already finished" once Wait has run.
+// Before the reaper existed this function reported a runtime that had terminated
+// itself hours earlier as alive (mxcli-formula1 FINDINGS §60). Removing watchExit
+// silently breaks that on POSIX. On Windows Signal(0) is not supported at all, so
+// processAlive uses WaitForSingleObject instead (see procgroup_windows.go).
 func (rt *LocalRuntime) alive() bool {
 	if rt.cmd == nil || rt.cmd.Process == nil {
 		return false
 	}
-	return rt.cmd.Process.Signal(syscall.Signal(0)) == nil
+	return processAlive(rt.cmd.Process)
 }
 
 // Stop shuts the runtime down gracefully via the admin API, then terminates the

cmd/mxcli/docker/mxserve.go

@@ -385,12 +385,16 @@ func (s *ServeServer) Build(req BuildRequest) (*BuildResult, error) {
 	return &res, nil
 }
 
-// alive reports whether the serve process is still running (Linux: signal 0).
+// alive reports whether the serve process is still running.
+//
+// Delegates to processAlive: Signal(0) is a correct liveness test on POSIX but
+// returns EWINDOWS on Windows, where it made waitReady() treat a just-started
+// mxbuild as dead.
 func (s *ServeServer) alive() bool {
 	if s.cmd == nil || s.cmd.Process == nil {
 		return false
 	}
-	return s.cmd.Process.Signal(syscall.Signal(0)) == nil
+	return processAlive(s.cmd.Process)
 }
 
 // Log returns the captured mxbuild --serve output (for diagnostics).

cmd/mxcli/docker/procgroup_unix.go

@@ -46,3 +46,13 @@ func signalProcessGroup(p *os.Process, sig syscall.Signal) error {
 func killProcessGroup(p *os.Process) error {
 	return signalProcessGroup(p, syscall.SIGKILL)
 }
+
+// processAlive reports whether p is still running. On POSIX, signal 0 succeeds
+// for a live process; it also succeeds for an unreaped zombie, which is why the
+// callers that care (LocalRuntime) reap via watchExit.
+func processAlive(p *os.Process) bool {
+	if p == nil {
+		return false
+	}
+	return p.Signal(syscall.Signal(0)) == nil
+}

cmd/mxcli/docker/procgroup_windows.go

@@ -7,29 +7,88 @@ package docker
 import (
 	"os"
 	"os/exec"
+	"strconv"
 	"syscall"
 )
 
-// procgroup_windows.go provides no-op / single-process fallbacks for the
-// process-group helpers. Windows has no POSIX process groups; the warm loop is a
-// Linux-devcontainer feature, so this only needs to keep the package compiling
-// and preserve the prior single-PID signalling behaviour.
+// procgroup_windows.go provides the Windows implementations of the process
+// helpers. Windows has no POSIX process groups, so the "group" operations are
+// implemented by terminating the whole process tree with taskkill instead.
 
-// setProcessGroup is a no-op on Windows.
+// setProcessGroup is a no-op on Windows: there is no PGID to set. Tree teardown
+// is handled by signalProcessGroup / killProcessGroup below.
 func setProcessGroup(cmd *exec.Cmd) {}
 
-// signalProcessGroup signals the process itself (no group semantics on Windows).
+// signalProcessGroup terminates the process tree led by p.
+//
+// Windows has no signal semantics — os.Process.Signal only supports Kill and
+// returns EWINDOWS for everything else — so a "graceful signal" cannot be
+// delivered and the process would otherwise linger until the caller's grace
+// period expired. Terminating the tree here keeps shutdown prompt.
 func signalProcessGroup(p *os.Process, sig syscall.Signal) error {
 	if p == nil {
 		return nil
 	}
+	if err := killProcessTree(p.Pid); err == nil {
+		return nil
+	}
 	return p.Signal(sig)
 }
 
-// killProcessGroup force-terminates the process.
+// killProcessGroup force-terminates the process tree led by p.
+//
+// The tree, not just p: mxbuild.exe is a wrapper that launches a Deno web-ext
+// worker (modeler/tools/deno/win-x64/deno.exe) which inherits the stdout/stderr
+// pipe this package hands to exec.Cmd. Killing only the wrapper leaves that
+// worker alive holding the pipe, so cmd.Wait() never sees EOF and blocks
+// forever — which is why `mxcli run --local` used to hang at "Starting mxbuild
+// --serve..." with no error. taskkill /T reaps the grandchild too.
+//
+// The direct p.Kill() is only a fallback for when taskkill is unavailable: once
+// the tree kill has terminated p, calling Kill on it again returns "invalid
+// argument", which must not be reported as a teardown failure.
 func killProcessGroup(p *os.Process) error {
 	if p == nil {
 		return nil
 	}
+	if err := killProcessTree(p.Pid); err == nil {
+		return nil
+	}
 	return p.Kill()
 }
+
+// killProcessTree force-terminates pid and all of its descendants, returning the
+// taskkill error (or nil when pid <= 0, which is nothing to do).
+func killProcessTree(pid int) error {
+	if pid <= 0 {
+		return nil
+	}
+	cmd := exec.Command("taskkill", "/F", "/T", "/PID", strconv.Itoa(pid))
+	cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true}
+	return cmd.Run()
+}
+
+// processAlive reports whether p is still running.
+//
+// os.Process.Signal(syscall.Signal(0)) is NOT a liveness test on Windows: Go
+// returns EWINDOWS ("not supported by windows") for every signal except Kill, so
+// it reports a live process as dead. waitReady() relied on it, concluded mxbuild
+// had exited the instant it was launched, and the local loop never got past
+// "Starting mxbuild --serve...". WaitForSingleObject on a SYNCHRONIZE handle is
+// the real check: it returns WAIT_TIMEOUT while the process runs and
+// WAIT_OBJECT_0 once it has terminated (even before it is reaped).
+func processAlive(p *os.Process) bool {
+	if p == nil {
+		return false
+	}
+	h, err := syscall.OpenProcess(syscall.SYNCHRONIZE, false, uint32(p.Pid))
+	if err != nil {
+		return false
+	}
+	defer syscall.CloseHandle(h)
+	event, err := syscall.WaitForSingleObject(h, 0)
+	if err != nil {
+		return false
+	}
+	return event == syscall.WAIT_TIMEOUT
+}

cmd/mxcli/docker/procgroup_windows_test.go

@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: Apache-2.0
+
+//go:build windows
+
+package docker
+
+import (
+	"os"
+	"os/exec"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+)
+
+// These tests exist because `mxcli run --local` hung forever on Windows at
+// "Starting mxbuild --serve...". Two POSIX assumptions were behind it, both in
+// the process helpers this file exercises:
+//
+//  1. alive() used os.Process.Signal(syscall.Signal(0)). Go returns EWINDOWS
+//     ("not supported by windows") for every signal except Kill — even for a
+//     process that is very much alive — so waitReady() concluded mxbuild had
+//     exited the instant it was launched.
+//
+//  2. Stop() waited on cmd.Wait(), and mxbuild.exe is a wrapper that launches a
+//     Deno web-ext worker which inherits the stdout/stderr pipe exec.Cmd hands
+//     out. Killing only the wrapper (the old killProcessGroup did p.Kill())
+//     left the worker holding the pipe, so Wait() never saw EOF and blocked
+//     forever.
+//
+// The unix implementation is covered by procgroup_unix_test.go; this file is the
+// Windows half, and the CI job that runs it is what keeps it from regressing.
+
+// TestWindowsProcessHelper is not a test of its own: it is the re-exec target the
+// tests below use to get a real, controllable process tree on Windows (where
+// there is no `sh -c`). It exits before the testing framework prints anything.
+func TestWindowsProcessHelper(t *testing.T) {
+	switch os.Getenv("MXCLI_PROC_HELPER") {
+	case "":
+		return // normal `go test` run: this is not a test
+	case "sleep":
+		time.Sleep(60 * time.Second)
+		os.Exit(0)
+	case "spawn":
+		// A grandchild that inherits our stdout/stderr — the inherited pipe is
+		// exactly what kept cmd.Wait() blocked in the field.
+		gc := exec.Command("cmd", "/c", "ping", "-n", "60", "127.0.0.1")
+		gc.Stdout = os.Stdout
+		gc.Stderr = os.Stderr
+		if err := gc.Start(); err != nil {
+			os.Exit(3)
+		}
+		os.Stdout.WriteString("grandchild-started\n")
+		time.Sleep(60 * time.Second)
+		os.Exit(0)
+	}
+}
+
+// helperCmd re-execs this test binary in the requested helper mode.
+func helperCmd(t *testing.T, mode string) *exec.Cmd {
+	t.Helper()
+	cmd := exec.Command(os.Args[0], "-test.run=TestWindowsProcessHelper")
+	cmd.Env = append(os.Environ(), "MXCLI_PROC_HELPER="+mode)
+	return cmd
+}
+
+// processAlive must report a running process as alive. The CONTROL in the middle
+// is the point: it asserts that the API processAlive replaces really is unusable
+// on Windows, so nobody "simplifies" the workaround back into the bug.
+func TestProcessAlive_ReportsRunningProcess(t *testing.T) {
+	cmd := helperCmd(t, "sleep")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start helper: %v", err)
+	}
+	t.Cleanup(func() { _ = cmd.Process.Kill() })
+
+	if !processAlive(cmd.Process) {
+		t.Fatal("processAlive() = false for a running process")
+	}
+	if err := cmd.Process.Signal(syscall.Signal(0)); err == nil {
+		t.Log("note: os.Process.Signal(0) succeeded on Windows; " +
+			"processAlive's WaitForSingleObject may no longer be required")
+	}
+
+	_ = cmd.Process.Kill()
+	_ = cmd.Wait()
+	if processAlive(cmd.Process) {
+		t.Fatal("processAlive() = true for an exited process")
+	}
+}
+
+func TestProcessAlive_FalseWhenNilOrExited(t *testing.T) {
+	if processAlive(nil) {
+		t.Fatal("processAlive(nil) = true")
+	}
+	cmd := exec.Command("cmd", "/c", "exit", "0")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start: %v", err)
+	}
+	_ = cmd.Wait()
+	if processAlive(cmd.Process) {
+		t.Fatal("processAlive() = true for an exited process")
+	}
+}
+
+// ServeServer.alive() is the exact call waitReady() makes. Before the fix it
+// returned false for a live mxbuild, which is what aborted the local boot.
+func TestServeServer_AliveTracksProcess(t *testing.T) {
+	if (&ServeServer{}).alive() {
+		t.Fatal("alive() = true with no process")
+	}
+
+	cmd := helperCmd(t, "sleep")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start helper: %v", err)
+	}
+	s := &ServeServer{cmd: cmd, log: &syncBuffer{}}
+
+	if !s.alive() {
+		t.Fatal("ServeServer.alive() = false for a running process " +
+			"(the Windows Signal(0) bug that aborted the local boot)")
+	}
+
+	_ = cmd.Process.Kill()
+	_ = cmd.Wait()
+	if s.alive() {
+		t.Fatal("ServeServer.alive() = true after the process exited")
+	}
+}
+
+// LocalRuntime.alive() shares the same helper and the same fix.
+func TestLocalRuntime_AliveTracksProcess(t *testing.T) {
+	cmd := helperCmd(t, "sleep")
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start helper: %v", err)
+	}
+	rt := &LocalRuntime{cmd: cmd, log: &syncBuffer{}}
+
+	if !rt.alive() {
+		t.Fatal("LocalRuntime.alive() = false for a running process")
+	}
+
+	_ = cmd.Process.Kill()
+	_ = cmd.Wait()
+	if rt.alive() {
+		t.Fatal("LocalRuntime.alive() = true after the process exited")
+	}
+}
+
+// TestKillProcessGroup_ReapsGrandchildAndUnblocksWait is the regression test for
+// the hang. The helper spawns a grandchild that inherits the stdout pipe; the old
+// single-PID kill left it alive, so cmd.Wait() blocked forever (there was no
+// error, no exit — just a hung `mxcli run --local`). A tree kill closes the pipe
+// and Wait() returns.
+//
+// It would have failed on the pre-fix code: p.Kill() terminates only the helper,
+// the grandchild keeps the write end open, and Wait() never returns.
+func TestKillProcessGroup_ReapsGrandchildAndUnblocksWait(t *testing.T) {
+	var log syncBuffer
+	cmd := helperCmd(t, "spawn")
+	cmd.Stdout = &log
+	cmd.Stderr = &log
+	setProcessGroup(cmd)
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("start helper: %v", err)
+	}
+	t.Cleanup(func() { _ = killProcessGroup(cmd.Process) })
+
+	// Wait for the grandchild to be running and holding the inherited pipe.
+	deadline := time.Now().Add(15 * time.Second)
+	for !strings.Contains(log.String(), "grandchild-started") {
+		if time.Now().After(deadline) {
+			t.Fatalf("helper never reported its grandchild; output so far: %q", log.String())
+		}
+		time.Sleep(50 * time.Millisecond)
+	}
+
+	done := make(chan error, 1)
+	go func() { done <- cmd.Wait() }()
+
+	if err := killProcessGroup(cmd.Process); err != nil {
+		t.Fatalf("killProcessGroup: %v", err)
+	}
+
+	select {
+	case <-done:
+	case <-time.After(20 * time.Second):
+		t.Fatal("cmd.Wait() did not return after killProcessGroup: a surviving " +
+			"grandchild still holds the stdout pipe (the old single-PID kill hung here)")
+	}
+}