Skip to content

Commit dba5103

Browse files
committed
fix: support Windows executable resolution, process-tree termination, and process_is_alive Win32 API
1 parent d672a86 commit dba5103

7 files changed

Lines changed: 405 additions & 24 deletions

File tree

‎src/yoke/process.py‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
"""Cross-platform process helpers for provider CLIs."""
2+
3+
from __future__ import annotations
4+
5+
import asyncio
6+
import contextlib
7+
import os
8+
import shutil
9+
import signal
10+
import subprocess
11+
from pathlib import Path
12+
13+
IS_WINDOWS = os.name == "nt"
14+
15+
16+
def path_for_provider(path: str | Path) -> str:
17+
"""Serialize a filesystem path for provider APIs with stable separators."""
18+
return Path(path).as_posix()
19+
20+
21+
def resolve_executable(command: str) -> str | None:
22+
"""Resolve a command name to an executable path on the current platform.
23+
24+
On Windows this finds npm ``.CMD``/``.BAT`` shims that
25+
``asyncio.create_subprocess_exec("codex", ...)`` cannot launch by bare
26+
name.
27+
28+
A bare name always goes through ``PATH``. Only a command written as a path
29+
is read from the filesystem, so a file named ``codex`` in the working
30+
directory cannot shadow the real executable.
31+
"""
32+
if _is_path_like(command):
33+
# Returned verbatim: normalising through Path would strip a leading
34+
# ``./``, turning an explicit relative path back into a bare name that
35+
# the exec call would then look up on PATH.
36+
return command if Path(command).is_file() else None
37+
return shutil.which(command)
38+
39+
40+
def _is_path_like(command: str) -> bool:
41+
"""Return whether a command names a filesystem path rather than a bare name."""
42+
separators = (os.sep, os.altsep) if os.altsep else (os.sep,)
43+
return any(separator in command for separator in separators)
44+
45+
46+
def popen_start_new_session() -> bool:
47+
"""Return whether new process groups are safe for this platform."""
48+
# Windows process groups do not match POSIX killpg semantics and can leave
49+
# npm shim trees behind when only the wrapper pid is terminated.
50+
return not IS_WINDOWS
51+
52+
53+
def process_is_alive(pid: int) -> bool:
54+
"""Return whether a PID currently refers to a live process."""
55+
if pid <= 0:
56+
return False
57+
if pid == os.getpid():
58+
return True
59+
if IS_WINDOWS:
60+
return _windows_process_is_alive(pid)
61+
try:
62+
os.kill(pid, 0)
63+
except ProcessLookupError:
64+
return False
65+
except PermissionError:
66+
return True
67+
return True
68+
69+
70+
def _windows_process_is_alive(pid: int) -> bool:
71+
import ctypes
72+
from ctypes import wintypes
73+
74+
kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
75+
process_query_limited_information = 0x1000
76+
still_active = 259
77+
handle = kernel32.OpenProcess(process_query_limited_information, 0, pid)
78+
if not handle:
79+
return False
80+
try:
81+
exit_code = wintypes.DWORD()
82+
if kernel32.GetExitCodeProcess(handle, ctypes.byref(exit_code)) == 0:
83+
return False
84+
return int(exit_code.value) == still_active
85+
finally:
86+
kernel32.CloseHandle(handle)
87+
88+
89+
def kill_process_tree(pid: int) -> None:
90+
"""Force-terminate a process and its descendants."""
91+
if pid <= 0:
92+
return
93+
if IS_WINDOWS:
94+
creationflags = getattr(subprocess, "CREATE_NO_WINDOW", 0)
95+
subprocess.run(
96+
["taskkill", "/PID", str(pid), "/T", "/F"],
97+
check=False,
98+
capture_output=True,
99+
creationflags=creationflags,
100+
)
101+
return
102+
if _kill_process_group(pid):
103+
return
104+
with contextlib.suppress(ProcessLookupError, PermissionError):
105+
os.kill(pid, signal.SIGKILL)
106+
107+
108+
def _kill_process_group(pid: int) -> bool:
109+
"""Kill the process group led by ``pid``, if it is safe to do so.
110+
111+
Children started with ``start_new_session=True`` lead their own group, so
112+
signalling the group reaches descendants that a bare ``os.kill`` would
113+
orphan. A child sharing this process's group is skipped: signalling that
114+
group would kill the caller too.
115+
"""
116+
try:
117+
group = os.getpgid(pid)
118+
except (ProcessLookupError, PermissionError, OSError):
119+
return False
120+
if group in (os.getpgid(0), 0):
121+
return False
122+
try:
123+
os.killpg(group, signal.SIGKILL)
124+
except (ProcessLookupError, PermissionError, OSError):
125+
return False
126+
return True
127+
128+
129+
async def terminate_asyncio_process(process: asyncio.subprocess.Process) -> None:
130+
"""Terminate an asyncio subprocess, including Windows npm shim trees."""
131+
if process.returncode is not None or process.pid is None:
132+
return
133+
if IS_WINDOWS:
134+
await asyncio.to_thread(kill_process_tree, process.pid)
135+
try:
136+
await asyncio.wait_for(process.wait(), timeout=2)
137+
except TimeoutError:
138+
process.kill()
139+
await process.wait()
140+
return
141+
process.kill()
142+
await process.wait()
143+
144+
145+
def terminate_popen(process: subprocess.Popen[str]) -> None:
146+
"""Terminate a ``subprocess.Popen`` process tree."""
147+
if process.poll() is not None or process.pid is None:
148+
return
149+
kill_process_tree(process.pid)
150+
try:
151+
process.wait(timeout=2)
152+
except subprocess.TimeoutExpired:
153+
process.kill()
154+
with contextlib.suppress(subprocess.TimeoutExpired):
155+
process.wait(timeout=0.5)

‎src/yoke/providers/codex_app/process.py‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@
1616
from pydantic import JsonValue, TypeAdapter, ValidationError
1717

1818
from yoke.errors import YokeError
19+
from yoke.process import (
20+
IS_WINDOWS,
21+
popen_start_new_session,
22+
resolve_executable,
23+
terminate_popen,
24+
)
1925
from yoke.providers.codex_app.fields import JsonObject, as_record, string_field
2026

2127
JSON_VALUE = TypeAdapter(JsonValue)
@@ -39,20 +45,23 @@ def start(
3945
cwd: Path,
4046
env: dict[str, str] | None,
4147
) -> JsonRpcLineProcess:
48+
executable = resolve_executable(command)
49+
if executable is None:
50+
raise FileNotFoundError(command)
4251
process_env = dict(os.environ)
4352
process_env.setdefault("YOKE_INTERNAL_SESSION", "1")
4453
if env is not None:
4554
process_env.update(env)
4655
child = subprocess.Popen(
47-
(command, *args),
56+
(executable, *args),
4857
cwd=cwd,
4958
env=process_env,
5059
text=True,
5160
stdin=subprocess.PIPE,
5261
stdout=subprocess.PIPE,
5362
stderr=subprocess.PIPE,
5463
bufsize=1,
55-
start_new_session=True,
64+
start_new_session=popen_start_new_session(),
5665
)
5766
return cls(child)
5867

@@ -87,6 +96,9 @@ def read_until(self, deadline: float, timeout_label: str) -> JsonObject:
8796
def terminate(self) -> None:
8897
if self.child.poll() is not None:
8998
return
99+
if IS_WINDOWS:
100+
terminate_popen(self.child)
101+
return
90102
try:
91103
os.killpg(os.getpgid(self.child.pid), signal.SIGTERM)
92104
except (AttributeError, ProcessLookupError, PermissionError, OSError):

‎src/yoke/providers/codex_cli.py‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
from typing import Any
1212

1313
from yoke.errors import YokeError
14+
from yoke.process import resolve_executable
1415

1516
ORIGINATOR_ENV = "CODEX_INTERNAL_ORIGINATOR_OVERRIDE"
1617
YOKE_ORIGINATOR = "yoke_python"
@@ -47,7 +48,10 @@ async def run(
4748
) -> AsyncIterator[dict[str, Any]]:
4849
schema_path: Path | None = None
4950
try:
50-
args = [self.executable, "exec", "--json", "--cd", str(cwd)]
51+
executable = resolve_executable(self.executable)
52+
if executable is None:
53+
raise FileNotFoundError(self.executable)
54+
args = [executable, "exec", "--json", "--cd", str(cwd)]
5155
if model:
5256
args.extend(["--model", model])
5357
if sandbox:

‎src/yoke/providers/runtime_deployment.py‎

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
from yoke.errors import YokeError
1313
from yoke.models import Agent, Provider, Skill
14+
from yoke.process import process_is_alive
1415
from yoke.providers.codex_agents import (
1516
codex_agent_name,
1617
codex_agent_toml,
@@ -117,20 +118,6 @@ def runtime_owner_pid(name: str) -> int | None:
117118
return pid if pid > 0 else None
118119

119120

120-
def process_is_alive(pid: int) -> bool:
121-
if pid == os.getpid():
122-
return True
123-
try:
124-
os.kill(pid, 0)
125-
except ProcessLookupError:
126-
return False
127-
except PermissionError:
128-
return True
129-
except OSError:
130-
return False
131-
return True
132-
133-
134121
def _write_codex(agent: Agent, deployment: RuntimeDeployment) -> None:
135122
agents_dir = deployment.root / "agents"
136123
entries, role_maps = _codex_roles(agent)

‎src/yoke/readiness.py‎

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import os
77
from dataclasses import dataclass
88

9+
from yoke.process import resolve_executable, terminate_asyncio_process
10+
911

1012
@dataclass(frozen=True)
1113
class CommandCheck:
@@ -29,11 +31,17 @@ async def run_command(
2931
) -> CommandCheck:
3032
"""Run one local readiness command."""
3133

34+
if not args:
35+
raise ValueError("run_command requires at least one argument")
36+
executable = resolve_executable(args[0])
37+
if executable is None:
38+
raise FileNotFoundError(args[0])
3239
process_env = dict(os.environ)
3340
if env is not None:
3441
process_env.update(env)
3542
process = await asyncio.create_subprocess_exec(
36-
*args,
43+
executable,
44+
*args[1:],
3745
stdout=asyncio.subprocess.PIPE,
3846
stderr=asyncio.subprocess.PIPE,
3947
env=process_env,
@@ -44,8 +52,7 @@ async def run_command(
4452
timeout=timeout_seconds,
4553
)
4654
except TimeoutError:
47-
process.kill()
48-
await process.wait()
55+
await terminate_asyncio_process(process)
4956
raise
5057
return CommandCheck(
5158
code=process.returncode or 0,
@@ -59,3 +66,12 @@ def first_line(value: str) -> str:
5966

6067
lines = value.splitlines()
6168
return lines[0] if lines else ""
69+
70+
71+
__all__ = [
72+
"CommandCheck",
73+
"first_line",
74+
"resolve_executable",
75+
"run_command",
76+
"terminate_asyncio_process",
77+
]

0 commit comments

Comments
 (0)