Skip to content

Commit cc1d60d

Browse files
DeusDatanoctrex
andcommitted
fix(windows): suppress the console window on the last spawn site (#1427)
#1448 set CREATE_NO_WINDOW on the subprocess spawn and on cbm_popen_isolated, but cbm_exec_no_shell (compat_fs.c) still passed a bare 0 for dwCreationFlags. That is the helper behind git, codesign and open, so the most frequently hit path kept flashing a console — and under a stdio MCP session with auto_watch those windows steal focus while the user is typing. Reported by @noctrex on #1448 after verifying the merged fix. Also adds the contract nobody had. The flag has to be set per call site, the four sites do not resemble each other, and reviewing "does this PR add the flag" says nothing about the sites a PR does not touch — which is exactly how the third one survived a fix applied twice: daemon/bootstrap.c had it from the start (DETACHED_PROCESS | ... | flag) subprocess.c added by #1448 (flags variable) compat_fs.c :313 added by #1448 (inline literal) compat_fs.c :689 missed by #1448 (bare 0) tests/test_spawn_no_window_contract.sh asserts the whole-tree property instead, so a fifth spawn site cannot be added without it. The contract strips C comments before matching, and that is load-bearing rather than tidiness: the first version of it PASSED with the fix reverted, because the explanatory comment above the fixed call contains the flag name and satisfied the match. A contract that can be satisfied by prose about the contract is a false guard. Verified in both directions afterwards — green with the fix, and red naming compat_fs.c:696 with the fix reverted and that comment still in place. Co-Authored-By: noctrex <noctrex@users.noreply.github.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 98f292e commit cc1d60d

3 files changed

Lines changed: 136 additions & 1 deletion

File tree

‎scripts/test.sh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,9 @@ bash "$ROOT/tests/test_parallel_harness_contract.sh"
230230
echo "=== Step 0j: venue parity contract (one harness, every venue) ==="
231231
bash "$ROOT/tests/test_venue_parity_contract.sh"
232232

233+
echo "=== Step 0k: spawn console-window contract (#1427) ==="
234+
bash "$ROOT/tests/test_spawn_no_window_contract.sh"
235+
233236
# Verify compiler supports target arch
234237
verify_compiler "$CC"
235238

‎src/foundation/compat_fs.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,14 @@ int cbm_exec_no_shell(const char *const *argv) {
686686
memset(&pi, 0, sizeof(pi));
687687
si.cb = sizeof(si);
688688

689-
if (!CreateProcessW(NULL, cmdline, NULL, NULL, FALSE, 0, NULL, NULL, &si, &pi)) {
689+
/* CREATE_NO_WINDOW: the third and last spawn site that still needed it
690+
* (#1427). Without it every helper routed through here — git, codesign,
691+
* open — flashes a console window, and under a stdio MCP session with
692+
* auto_watch those steal focus while the user is typing. The other three
693+
* CreateProcessW sites already set it: subprocess.c and cbm_popen_isolated
694+
* via #1448, and the detached daemon spawn in daemon/bootstrap.c, which has
695+
* had it since it was written. */
696+
if (!CreateProcessW(NULL, cmdline, NULL, NULL, FALSE, CREATE_NO_WINDOW, NULL, NULL, &si, &pi)) {
690697
free(cmdline);
691698
return CBM_NOT_FOUND;
692699
}
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
#!/usr/bin/env bash
2+
# Contract: every CreateProcessW spawn must suppress its console window.
3+
#
4+
# This exists because the fix for #1427 was applied THREE times and still missed
5+
# a site. The reported symptom is a console window stealing focus mid-typing on a
6+
# stdio MCP session with auto_watch. The flag that prevents it, CREATE_NO_WINDOW,
7+
# has to be set at each spawn independently, and the sites do not look alike:
8+
#
9+
# daemon/bootstrap.c had it from the start (DETACHED_PROCESS | ... | CREATE_NO_WINDOW)
10+
# subprocess.c added by #1448 (flags variable)
11+
# compat_fs.c :313 added by #1448 (inline literal, popen path)
12+
# compat_fs.c :689 MISSED by #1448 (dwCreationFlags was a bare 0)
13+
#
14+
# The missed one is the helper that runs git / codesign / open, i.e. the one a
15+
# user meets most often. Nobody was wrong to miss it: it is a different function
16+
# from the popen site in the same file, and reviewing "does the PR add the flag"
17+
# says nothing about the sites the PR does not touch. Only a whole-tree property
18+
# catches that, so this asserts the property rather than any one call.
19+
#
20+
# Python, not shell: this needs the 6th argument of a call spanning lines, and
21+
# the enclosing function's flags variable when that argument is not a literal.
22+
set -euo pipefail
23+
24+
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
25+
cd "$ROOT"
26+
27+
python3 - "$ROOT" <<'PY'
28+
import pathlib
29+
import re
30+
import sys
31+
32+
root = pathlib.Path(sys.argv[1])
33+
34+
35+
def strip_comments(text):
36+
"""Blank out C comments, preserving line count and column positions.
37+
38+
Mandatory, not tidiness: the first version of this contract matched the flag
39+
name anywhere near the call, so the explanatory COMMENT sitting above the
40+
fixed call site satisfied it — the test passed with the fix reverted. A
41+
contract that can be satisfied by prose about the contract is a false guard.
42+
"""
43+
out = []
44+
i, n = 0, len(text)
45+
while i < n:
46+
two = text[i:i + 2]
47+
if two == "/*":
48+
j = text.find("*/", i + 2)
49+
j = n if j < 0 else j + 2
50+
out.append("".join(c if c == "\n" else " " for c in text[i:j]))
51+
i = j
52+
elif two == "//":
53+
j = text.find("\n", i)
54+
j = n if j < 0 else j
55+
out.append(" " * (j - i))
56+
i = j
57+
else:
58+
out.append(text[i])
59+
i += 1
60+
return "".join(out)
61+
62+
# A site may opt out only with a justified, at-the-case entry (O10): the exact
63+
# "file:function" plus WHY. Empty on purpose — every current site sets the flag.
64+
ALLOWLIST = {
65+
# "src/foo/bar.c:some_function": "why a console window is required here",
66+
}
67+
68+
FLAG = "CREATE_NO_WINDOW"
69+
failures = []
70+
checked = 0
71+
72+
for path in sorted(root.glob("src/**/*.c")):
73+
raw = path.read_text(encoding="utf-8", errors="replace")
74+
if "CreateProcessW(" not in raw:
75+
continue
76+
# Comments are blanked FIRST, so neither the call scan nor the flag check can
77+
# be satisfied by prose. Line numbers still map to the real file.
78+
text = strip_comments(raw)
79+
lines = text.splitlines()
80+
for idx, line in enumerate(lines):
81+
if "CreateProcessW(" not in line:
82+
continue
83+
checked += 1
84+
85+
# Enclosing function: nearest preceding line that starts in column 0 and
86+
# looks like a definition. Good enough — these files are plain C.
87+
func = "<unknown>"
88+
for back in range(idx, -1, -1):
89+
m = re.match(r"^(?:static\s+)?[A-Za-z_][\w \t*]*\b(\w+)\s*\(", lines[back])
90+
if m and not lines[back].lstrip().startswith(("*", "//", "/*")):
91+
func = m.group(1)
92+
break
93+
rel = path.relative_to(root).as_posix()
94+
key = f"{rel}:{func}"
95+
if key in ALLOWLIST:
96+
continue
97+
98+
# The call may wrap across lines; take a window around it, plus the
99+
# enclosing function body, so a `DWORD flags = ...` assignment counts.
100+
call_window = "\n".join(lines[idx:idx + 4])
101+
body_start = max(0, idx - 60)
102+
body_window = "\n".join(lines[body_start:idx + 4])
103+
104+
if FLAG in call_window or FLAG in body_window:
105+
continue
106+
failures.append(
107+
f"{rel}:{idx + 1} ({func}) spawns without {FLAG}\n"
108+
f" {line.strip()}"
109+
)
110+
111+
if checked == 0:
112+
print("FAIL: no CreateProcessW call sites found — this contract has stopped "
113+
"checking anything (did the spawn layer move?)")
114+
sys.exit(1)
115+
116+
if failures:
117+
print(f"FAIL: {len(failures)} CreateProcessW site(s) can pop a console window (#1427):\n")
118+
for f in failures:
119+
print(f" - {f}")
120+
print(f"\nAdd {FLAG} to dwCreationFlags. If a site genuinely needs a console, "
121+
"add it to ALLOWLIST in this file with the reason.")
122+
sys.exit(1)
123+
124+
print(f"OK: all {checked} CreateProcessW site(s) set {FLAG}")
125+
PY

0 commit comments

Comments
 (0)