diff --git a/.github/workflows/python-sdk.yml b/.github/workflows/python-sdk.yml index 44fda178d..219ceb5d0 100644 --- a/.github/workflows/python-sdk.yml +++ b/.github/workflows/python-sdk.yml @@ -23,7 +23,11 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest] - python: ["3.10", "3.13"] + python: ["3.10", "3.13", "3.14"] + include: + # Launcher resolution and option tests; fake-CLI process tests are POSIX-only. + - os: windows-latest + python: "3.13" runs-on: ${{ matrix.os }} defaults: run: @@ -38,3 +42,6 @@ jobs: - run: uv run ruff format --check . - run: uv run mypy - run: uv run pytest -W error + env: + # Fail on implicit text encodings, which use the locale code page on Windows. + PYTHONWARNDEFAULTENCODING: "1" diff --git a/packages/code-sdk-python/CHANGELOG.md b/packages/code-sdk-python/CHANGELOG.md index d84e0b599..385c27548 100644 --- a/packages/code-sdk-python/CHANGELOG.md +++ b/packages/code-sdk-python/CHANGELOG.md @@ -20,6 +20,23 @@ versioned independently of the MiniMax Code CLI and of the TypeScript SDK. The f - SDK errors survive pickling: `MCodeRunError` can be unpickled, and `ThreadBusyError` keeps its message instead of repeating it. - `repr(MCodeOptions(...))` no longer prints `env` values, which could expose secrets in logs. +- Cancellation sends SIGTERM instead of SIGINT. After Ctrl+C in a terminal had already delivered + SIGINT to the CLI, the SDK's second SIGINT hit the CLI's default action and killed it before it + could persist the Turn and write `exec.completed`. +- Cancellation waits for the CLI's 60 second shutdown budget before SIGKILL (was 10 seconds), and + the `timeout_ms` backstop fires 90 seconds after `timeout_ms` (was 30), matching the + TypeScript SDK. The shorter values could kill a CLI that was still shutting down cleanly. +- `exec.completed` ends the Turn: later output is ignored and a CLI that does not exit within + 5 seconds is killed. A background process that inherited the CLI's stdout or stderr no longer + keeps `thread.run()` waiting until it exits; after the CLI exits, remaining output is read for + at most 2 seconds. +- A Thread pins `ThreadOptions.cwd` (default: the current directory) when it is created. A later + `os.chdir()` used to move the next Turn to another workspace, which the CLI rejects for an + existing Session. +- Windows: `mcode.cmd` launchers from the official installer and npm / pnpm are no longer run + through cmd.exe. The SDK runs the JS entry and Node runtime behind them, so arguments are not + reinterpreted by cmd.exe and cancelling stops the CLI itself rather than only cmd.exe. + `MCODE_BIN` and `PATH` are looked up case-insensitively on Windows. ### Documentation diff --git a/packages/code-sdk-python/README.md b/packages/code-sdk-python/README.md index fa9f00775..73d1349ee 100644 --- a/packages/code-sdk-python/README.md +++ b/packages/code-sdk-python/README.md @@ -21,7 +21,10 @@ The SDK is in beta, so installers need `--pre` (or an exact version such as `minimax-code-sdk==0.1.0b1`) until `0.1.0` is released. Import it as `minimax_code`. The CLI is found from `MCodeOptions(bin=...)`, then the `MCODE_BIN` environment variable, -then `PATH`. +then `PATH`. The SDK never starts it through a shell. On Windows, the `mcode.cmd` launcher +written by the official installer or by npm / pnpm is followed to the JS entry and Node runtime +it runs (the installer's release pointer is read again before every Turn, so a CLI self-update +is picked up); an unrecognized `.cmd` file is rejected, and you can pass the JS entry as `bin`. ## Quick start @@ -51,7 +54,8 @@ asyncio.run(main()) ### Resuming a Session A Session belongs to the workspace it was created in. Pass the same `cwd` to `resume_thread()`; -when `cwd` is omitted, the current working directory is used. A different workspace fails with +when `cwd` is omitted, the current working directory at that call is used. A Thread pins its +`cwd` when it is created, so a later `os.chdir()` does not affect it. A different workspace fails with `MCodeRunError` ("Session ... belongs to workspace ..."), and retrying does not help. ```python @@ -78,13 +82,19 @@ event; iteration raises only for process or protocol failures. ### Cancellation -- `streamed.cancel()` sends SIGINT; the stream ends with the CLI's final events. +- `streamed.cancel()` sends SIGTERM, the CLI's cancel signal; the stream ends with the CLI's + final events. SIGTERM is used instead of SIGINT because Ctrl+C in a terminal already sends + SIGINT to the CLI, and a second SIGINT would skip its graceful shutdown. - Leaving the `async for` early (`break` or an exception in the loop body), or cancelling the - task running `thread.run()`, stops the CLI (SIGINT, then SIGKILL after a grace period). The - next Turn on the same Thread waits for that shutdown instead of raising `ThreadBusyError`; - `async with streamed:` or `await streamed.aclose()` stops it deterministically. -- `RunOptions(timeout_ms=...)` is enforced by the CLI. If the CLI is still running 30 seconds - after `timeout_ms`, the SDK kills it and raises `MCodeRunError` with `status="timeout"`. + task running `thread.run()`, stops the CLI: SIGTERM, then SIGKILL if it is still running after + the CLI's 60 second shutdown budget. The next Turn on the same Thread waits for that shutdown + instead of raising `ThreadBusyError`; `async with streamed:` or `await streamed.aclose()` + stops it deterministically. On Windows, cancellation stops the CLI immediately. +- `RunOptions(timeout_ms=...)` is enforced by the CLI. If the CLI is still running 90 seconds + after `timeout_ms` (its shutdown budget plus startup time), the SDK kills it and raises + `MCodeRunError` with `status="timeout"`. +- `exec.completed` ends the Turn. A CLI that keeps running afterwards is stopped after 5 seconds, + and a background process that inherited the CLI's output does not keep the Turn open. ### Blocking API diff --git a/packages/code-sdk-python/pyproject.toml b/packages/code-sdk-python/pyproject.toml index 6ccacf3f7..b764577c5 100644 --- a/packages/code-sdk-python/pyproject.toml +++ b/packages/code-sdk-python/pyproject.toml @@ -15,6 +15,11 @@ keywords = ["minimax", "minimax-code", "mcode", "agent", "coding-agent", "sdk"] classifiers = [ "Development Status :: 4 - Beta", "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.10", + "Programming Language :: Python :: 3.11", + "Programming Language :: Python :: 3.12", + "Programming Language :: Python :: 3.13", + "Programming Language :: Python :: 3.14", "Operating System :: OS Independent", "Typing :: Typed", ] diff --git a/packages/code-sdk-python/src/minimax_code/_cli.py b/packages/code-sdk-python/src/minimax_code/_cli.py index 1d5585983..8f9cda83e 100644 --- a/packages/code-sdk-python/src/minimax_code/_cli.py +++ b/packages/code-sdk-python/src/minimax_code/_cli.py @@ -1,4 +1,7 @@ -"""Locates and version-checks the ``mcode`` CLI.""" +"""Locates and version-checks the ``mcode`` CLI. Mirrors ``packages/code-sdk/src/cli-resolver.ts`` +and the Windows launcher handling of ``project-bin.ts``: the SDK never starts the CLI through a +shell, so Windows ``.cmd`` launchers are followed to the JS entry and Node runtime they run. +""" from __future__ import annotations @@ -6,6 +9,7 @@ import os import re import shutil +import sys from collections.abc import Mapping from dataclasses import dataclass @@ -13,35 +17,149 @@ MIN_CLI_VERSION = (0, 6, 3) _JS_SUFFIXES = (".js", ".mjs", ".cjs") +#: Windows files that CreateProcess runs directly, without cmd.exe. +_WINDOWS_NATIVE = (".exe", ".com") +#: Lines of the Windows root launcher written by the official installer and CLI self-update. +_VERSIONED_POINTER_LINE = 'SET /P MCODE_RELEASE=<"%~dp0current"' +_VERSIONED_CALL_LINE = 'CALL "%~dp0releases\\%MCODE_RELEASE%\\.mcode-launcher.cmd"' +#: Same release-key rule the root launcher enforces before following the pointer. +_RELEASE_KEY = re.compile(r"[0-9A-Za-z][0-9A-Za-z._-]*") +#: JS entry in an npm / pnpm ``.cmd`` shim or an installer release launcher. +_CMD_SHIM_ENTRY = re.compile(r'"%~?dp0%?\\?([^"%]+\.(?:c|m)?js)"', re.IGNORECASE) +#: JS entry in an npm / pnpm POSIX shell or PowerShell shim. +_SHELL_SHIM_ENTRY = re.compile(r'"\$basedir[\\/]([^"$]+\.(?:c|m)?js)"') +#: The pinned runtime of an installer release launcher: ``"" "%~dp0" %*``. +_CMD_RUNTIME = re.compile(r'^"([^"%]+)"[ \t]+"%~dp0', re.MULTILINE) +_SHIM_HEAD_BYTES = 4096 @dataclass(frozen=True) class ResolvedCli: + """How to start the CLI.""" + path: str + """The executable, or the JS entry that runs with Node.""" + node: str | None = None + """Node runtime for a JS entry; ``None`` uses ``node`` from ``PATH``.""" + launcher: str | None = None + """Windows installer root launcher. Its release pointer is followed again before every run, so + a CLI self-update between Turns is picked up.""" def command(self, args: list[str]) -> list[str]: - if self.path.endswith(_JS_SUFFIXES): - node = shutil.which("node") or "node" - return [node, self.path, *args] + if self.launcher is not None: + current = _follow_versioned_launcher(self.launcher) + if current is None: + raise MCodeNotFoundError( + f"{self.launcher} no longer points to a usable release. " + "Pass the CLI's JS entry as MCodeOptions(bin=...)." + ) + return current.command(args) + if self.path.lower().endswith(_JS_SUFFIXES): + return [self.node or shutil.which("node") or "node", self.path, *args] return [self.path, *args] -def resolve_cli(bin: str | None, env: Mapping[str, str]) -> ResolvedCli: +def resolve_cli( + bin: str | None, env: Mapping[str, str], platform: str = sys.platform +) -> ResolvedCli: """Explicit ``bin`` → ``$MCODE_BIN`` → ``mcode`` on ``PATH``.""" - explicit = bin or env.get("MCODE_BIN") or None + explicit = bin or _env(env, "MCODE_BIN", platform) or None if explicit: path = os.path.abspath(os.path.expanduser(explicit)) if not os.path.isfile(path): source = "bin" if bin else "MCODE_BIN" raise MCodeNotFoundError(f"mcode CLI from {source} does not exist: {path}") - return ResolvedCli(path) - found = shutil.which("mcode", path=env.get("PATH", os.defpath)) + return _from_executable(path, platform) + found = shutil.which("mcode", path=_env(env, "PATH", platform) or os.defpath) if not found: raise MCodeNotFoundError( "Could not find the mcode CLI on PATH. Install it with " f"`{MCODE_INSTALL_HINT}`, set MCODE_BIN, or pass MCodeOptions(bin=...)." ) - return ResolvedCli(found) + return _from_executable(found, platform) + + +def _from_executable(path: str, platform: str) -> ResolvedCli: + extension = os.path.splitext(path)[1].lower() + if extension in _JS_SUFFIXES or platform != "win32" or extension in _WINDOWS_NATIVE: + return ResolvedCli(path) + # A batch file or shell shim would need cmd.exe and its quoting rules, and cancelling it would + # stop only cmd.exe and leave the CLI running. Run the JS entry behind it instead. + try: + head = _read_head(path) + except OSError: + head = "" + if extension in (".cmd", ".bat"): + resolved = resolve_cmd_launcher(head, path) + else: + entry = _parse_shim(_SHELL_SHIM_ENTRY, head, path) + resolved = ResolvedCli(entry, node=_adjacent_node(path)) if entry else None + if resolved is None: + raise MCodeNotFoundError( + f"Found {path}, but could not determine the JS entry it runs. " + "Pass the CLI's JS entry as MCodeOptions(bin=...) or MCODE_BIN." + ) + return resolved + + +def resolve_cmd_launcher(content: str, launcher: str) -> ResolvedCli | None: + """Resolves a Windows ``.cmd`` launcher: an npm / pnpm shim, or the installer's root + launcher, whose release is looked up again before every run.""" + entry = _parse_shim(_CMD_SHIM_ENTRY, content, launcher) + if entry: + return ResolvedCli(entry, node=_adjacent_node(launcher)) + versioned = _VERSIONED_POINTER_LINE in content and _VERSIONED_CALL_LINE in content + if versioned and _follow_versioned_launcher(launcher) is not None: + return ResolvedCli(launcher, launcher=launcher) + return None + + +def _follow_versioned_launcher(launcher: str) -> ResolvedCli | None: + """``\\mcode.cmd`` reads the release key from ``\\current`` and calls + ``\\releases\\\\.mcode-launcher.cmd``, which runs a pinned Node runtime.""" + prefix = os.path.dirname(launcher) + try: + with open(os.path.join(prefix, "current"), encoding="utf-8") as handle: + release = handle.read().strip() + if not _RELEASE_KEY.fullmatch(release): + return None + release_launcher = os.path.join(prefix, "releases", release, ".mcode-launcher.cmd") + content = _read_head(release_launcher) + except OSError: + return None + entry = _parse_shim(_CMD_SHIM_ENTRY, content, release_launcher) + if entry is None: + return None + runtime = _CMD_RUNTIME.search(content) + node = runtime.group(1) if runtime and os.path.isfile(runtime.group(1)) else None + return ResolvedCli(entry, node=node) + + +def _parse_shim(pattern: re.Pattern[str], content: str, shim: str) -> str | None: + match = pattern.search(content) + if not match: + return None + parts = [part for part in re.split(r"[\\/]", match.group(1)) if part] + entry = os.path.normpath(os.path.join(os.path.dirname(shim), *parts)) + return entry if os.path.isfile(entry) else None + + +def _adjacent_node(shim: str) -> str | None: + """npm shims prefer a ``node.exe`` next to them over ``PATH``.""" + candidate = os.path.join(os.path.dirname(shim), "node.exe") + return candidate if os.path.isfile(candidate) else None + + +def _read_head(path: str) -> str: + with open(path, "rb") as handle: + return handle.read(_SHIM_HEAD_BYTES).decode("utf-8", "replace") + + +def _env(env: Mapping[str, str], key: str, platform: str) -> str | None: + """Environment names are case-insensitive on Windows (``Path``).""" + if platform != "win32": + return env.get(key) + return next((value for name, value in env.items() if name.upper() == key), None) def parse_version(text: str) -> tuple[int, int, int] | None: @@ -84,4 +202,4 @@ async def check_cli_version(cli: ResolvedCli, env: Mapping[str, str]) -> str: return ".".join(map(str, version)) -__all__ = ["ResolvedCli", "check_cli_version", "resolve_cli"] +__all__ = ["ResolvedCli", "check_cli_version", "resolve_cli", "resolve_cmd_launcher"] diff --git a/packages/code-sdk-python/src/minimax_code/client.py b/packages/code-sdk-python/src/minimax_code/client.py index 704bd8320..9e8d47b28 100644 --- a/packages/code-sdk-python/src/minimax_code/client.py +++ b/packages/code-sdk-python/src/minimax_code/client.py @@ -3,6 +3,7 @@ from __future__ import annotations import asyncio +import dataclasses import os from ._args import validate_thread_options @@ -27,23 +28,27 @@ def __init__(self, options: MCodeOptions | None = None) -> None: ) def start_thread(self, options: ThreadOptions | None = None) -> Thread: - """Starts a new Thread. The Session is created by its first Turn.""" + """Starts a new Thread. The Session is created by its first Turn. + + ``ThreadOptions.cwd`` defaults to the current working directory and is resolved now, so a + later ``os.chdir()`` does not move the Thread to another workspace. + """ options = options or ThreadOptions() validate_thread_options(options, resuming=False) - return Thread(self._host, options, None) + return Thread(self._host, _with_absolute_cwd(options), None) def resume_thread(self, thread_id: str, options: ThreadOptions | None = None) -> Thread: """Continues an existing Session, for example one persisted from ``thread.id``. Pass the same ``ThreadOptions.cwd`` the Session was created with. When ``cwd`` is omitted - the current working directory is used, and the CLI rejects a Session whose workspace - differs (``MCodeRunError`` with "belongs to workspace ..."). + the current working directory at this call is used, and the CLI rejects a Session whose + workspace differs (``MCodeRunError`` with "belongs to workspace ..."). """ if not isinstance(thread_id, str) or not thread_id.strip(): raise MCodeInvocationError("resume_thread requires a non-empty Session id.") options = options or ThreadOptions() validate_thread_options(options, resuming=True) - return Thread(self._host, options, thread_id.strip()) + return Thread(self._host, _with_absolute_cwd(options), thread_id.strip()) def _child_env(self) -> dict[str, str]: env = dict(self._options.env if self._options.env is not None else os.environ) @@ -64,3 +69,8 @@ async def _prepare_cli(self) -> ResolvedCli: # Only cached on success, so a later run works after installing the CLI. self._cli = cli return self._cli + + +def _with_absolute_cwd(options: ThreadOptions) -> ThreadOptions: + """Pins the workspace when the Thread is created; every Turn of a Session must use it.""" + return dataclasses.replace(options, cwd=os.path.abspath(options.cwd or os.getcwd())) diff --git a/packages/code-sdk-python/src/minimax_code/thread.py b/packages/code-sdk-python/src/minimax_code/thread.py index ea389898d..1cd3e7f14 100644 --- a/packages/code-sdk-python/src/minimax_code/thread.py +++ b/packages/code-sdk-python/src/minimax_code/thread.py @@ -33,8 +33,22 @@ from .types import ExecResult, RunOptions, ThreadEvent, ThreadOptions, Turn STDERR_TAIL_BYTES = 64 * 1024 -CANCEL_GRACE_S = 10.0 -BACKSTOP_EXTRA_S = 30.0 +#: CLI runtime shutdown budget. Cancellation waits at least this long before SIGKILL so a CLI that +#: is already shutting down can still persist the Turn and write ``exec.completed``. +CLI_SHUTDOWN_BUDGET_S = 60.0 +#: Slack for work the CLI does (access, Session and model resolution) before it starts its own +#: ``timeout_ms`` timer; the SDK's timer starts at spawn. +CLI_STARTUP_SLACK_S = 30.0 +#: Cancel signal → SIGKILL. Read at call time so tests can shorten it. +CANCEL_GRACE_S = CLI_SHUTDOWN_BUDGET_S +#: Extra time after ``timeout_ms`` before the SDK force-kills a CLI that did not stop itself. +BACKSTOP_EXTRA_S = CLI_SHUTDOWN_BUDGET_S + CLI_STARTUP_SLACK_S +#: After ``exec.completed``, how long a lingering CLI may take to exit before SIGKILL. +TERMINATE_GRACE_S = 5.0 +#: After the CLI exits, how long to keep reading stdout. A grandchild process that inherited the +#: pipe can keep it open indefinitely. +EXIT_DRAIN_S = 2.0 +_EXIT_POLL_S = 0.05 _STDOUT_LINE_LIMIT = 64 * 1024 * 1024 @@ -171,10 +185,21 @@ def __init__(self, proc: asyncio.subprocess.Process) -> None: self.proc = proc self.stderr_tail = "" self._stderr_task = asyncio.ensure_future(self._drain_stderr()) + self._exit_task = asyncio.ensure_future(self._watch_exit()) + self._drain_deadline: float | None = None self._interrupted = False #: Set when the ``timeout_ms`` backstop had to kill the CLI. self.backstop_fired = False + async def _watch_exit(self) -> None: + # ``Process.wait()`` resolves only after every pipe is closed, so a grandchild that + # inherited stdout or stderr would hold it. ``returncode`` is set once the CLI is reaped. + while self.proc.returncode is None: + await asyncio.sleep(_EXIT_POLL_S) + + async def wait_exited(self) -> None: + await asyncio.shield(self._exit_task) + async def _drain_stderr(self) -> None: assert self.proc.stderr is not None while chunk := await self.proc.stderr.read(65536): @@ -188,7 +213,30 @@ def kill_for_timeout(self) -> None: with contextlib.suppress(ProcessLookupError): self.proc.kill() + async def read_line(self) -> bytes: + """Next stdout line, ``b""`` at the end. Once the CLI has exited, the rest of its output is + read for at most ``EXIT_DRAIN_S``: a grandchild holding the pipe must not stall the Turn.""" + assert self.proc.stdout is not None + loop = asyncio.get_running_loop() + read = asyncio.ensure_future(self.proc.stdout.readline()) + try: + if self._drain_deadline is None: + await asyncio.wait({read, self._exit_task}, return_when=asyncio.FIRST_COMPLETED) + if read.done(): + return read.result() + self._drain_deadline = loop.time() + EXIT_DRAIN_S + remaining = max(0.0, self._drain_deadline - loop.time()) + done, _ = await asyncio.wait({read}, timeout=remaining) + return read.result() if done else b"" + finally: + if not read.done(): + read.cancel() + def interrupt(self) -> None: + """Asks the CLI to cancel. SIGTERM is the CLI's cancel signal, like SIGINT, but has its own + one-shot handler: after a terminal Ctrl+C already delivered SIGINT to the CLI's process + group, a second SIGINT would hit the default action and skip the graceful shutdown. On + Windows this is a hard kill.""" if self.proc.returncode is not None or self._interrupted: return self._interrupted = True @@ -196,20 +244,24 @@ def interrupt(self) -> None: if sys.platform == "win32": self.proc.terminate() else: - self.proc.send_signal(signal.SIGINT) + self.proc.send_signal(signal.SIGTERM) + + async def wait_exit(self, grace: float) -> None: + """Waits up to ``grace`` seconds for the CLI to exit, then kills it.""" + try: + await asyncio.wait_for(asyncio.shield(self._exit_task), timeout=grace) + except asyncio.TimeoutError: + with contextlib.suppress(ProcessLookupError): + self.proc.kill() + await asyncio.shield(self._exit_task) - async def shutdown(self, grace: float = CANCEL_GRACE_S) -> None: - """Interrupts a live CLI, then kills it after ``grace`` seconds.""" + async def shutdown(self, grace: float | None = None) -> None: + """Interrupts a live CLI, then kills it after ``grace`` seconds (``CANCEL_GRACE_S``).""" if self.proc.returncode is None: self.interrupt() - try: - await asyncio.wait_for(asyncio.shield(self.proc.wait()), timeout=grace) - except asyncio.TimeoutError: - with contextlib.suppress(ProcessLookupError): - self.proc.kill() - await self.proc.wait() + await self.wait_exit(CANCEL_GRACE_S if grace is None else grace) with contextlib.suppress(asyncio.TimeoutError): - await asyncio.wait_for(asyncio.shield(self._stderr_task), timeout=2) + await asyncio.wait_for(asyncio.shield(self._stderr_task), timeout=EXIT_DRAIN_S) # Close the pipes now, while the loop is alive; asyncio has no public API for this. transport = getattr(self.proc, "_transport", None) if transport is not None: @@ -368,7 +420,7 @@ async def _events( line_no = 0 while True: try: - raw = await child.proc.stdout.readline() + raw = await child.read_line() except ValueError as error: raise MCodeProtocolError("mcode stdout line exceeds the size limit.") from error if not raw: @@ -387,7 +439,14 @@ async def _events( completed = True if items.apply(event): yield event - await child.proc.wait() + if completed: + # exec.completed is terminal; anything the CLI prints afterwards is ignored. + break + if completed: + # A CLI that lingers after exec.completed is force-stopped. + await child.wait_exit(TERMINATE_GRACE_S) + else: + await child.wait_exited() await child.shutdown() state["exit_code"] = child.proc.returncode state["stderr"] = child.stderr_tail diff --git a/packages/code-sdk-python/tests/conftest.py b/packages/code-sdk-python/tests/conftest.py index f2573d0e8..f2bc46957 100644 --- a/packages/code-sdk-python/tests/conftest.py +++ b/packages/code-sdk-python/tests/conftest.py @@ -1,6 +1,7 @@ from __future__ import annotations import os +import sys from pathlib import Path import pytest @@ -10,6 +11,8 @@ @pytest.fixture def fake_env(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> dict[str, str]: + if sys.platform == "win32": + pytest.skip("the fake CLI is a POSIX script; Windows runs the resolver and option tests") record = tmp_path / "calls.jsonl" env = dict(os.environ) env.update({"MCODE_BIN": str(FAKE), "FAKE_MCODE_RECORD": str(record)}) diff --git a/packages/code-sdk-python/tests/fake_mcode.py b/packages/code-sdk-python/tests/fake_mcode.py index ebd682371..fdf6395e0 100755 --- a/packages/code-sdk-python/tests/fake_mcode.py +++ b/packages/code-sdk-python/tests/fake_mcode.py @@ -6,6 +6,7 @@ import json import os import signal +import subprocess import sys import time @@ -17,6 +18,20 @@ sys.exit(0) prompt = json.loads(sys.stdin.read())["prompt"] +pid_file = os.environ.get("FAKE_MCODE_PID") +if pid_file: + with open(pid_file, "w", encoding="utf-8") as fh: + fh.write(str(os.getpid())) + + +def note(text: str) -> None: + """Appends to FAKE_MCODE_LOG, for assertions about how the SDK stopped the CLI.""" + path = os.environ.get("FAKE_MCODE_LOG") + if path: + with open(path, "a", encoding="utf-8") as fh: + fh.write(text + "\n") + + record = os.environ.get("FAKE_MCODE_RECORD") if record: with open(record, "a", encoding="utf-8") as fh: @@ -78,23 +93,46 @@ def result(status: str, **extra: object) -> dict[str, object]: emit("future.event", payload=True) if scenario == "slow": + # Like the CLI: SIGINT and SIGTERM each have a one-shot cancel handler; a repeated signal hits + # the default action and kills the process before exec.completed. cancelled = False - def on_sigint(_sig: int, _frame: object) -> None: + def on_signal(sig: int, _frame: object) -> None: global cancelled + note(f"signal {signal.Signals(sig).name}") + signal.signal(sig, signal.SIG_DFL) cancelled = True - signal.signal(signal.SIGINT, on_sigint) + signal.signal(signal.SIGINT, on_signal) + signal.signal(signal.SIGTERM, on_signal) for _ in range(200): if cancelled: break time.sleep(0.05) + time.sleep(float(os.environ.get("FAKE_MCODE_SHUTDOWN_S", "0"))) emit("turn.failed", status="cancelled", durationMs=5) emit("exec.completed", result=result("cancelled")) + note("completed") sys.exit(130) -if scenario == "hang": # ignores SIGINT and never finishes: only SIGKILL stops it +if scenario == "hang": # ignores cancel signals and never finishes: only SIGKILL stops it signal.signal(signal.SIGINT, signal.SIG_IGN) + signal.signal(signal.SIGTERM, signal.SIG_IGN) + time.sleep(60) + sys.exit(0) + +if scenario in ("grandchild", "grandchild_crash"): + # A background process inherits stdout and stderr and outlives the CLI. + subprocess.Popen([sys.executable, "-c", "import time; time.sleep(10)"]) + if scenario == "grandchild_crash": + sys.exit(4) + emit("exec.completed", result=result("succeeded", output="ok")) + sys.exit(0) + +if scenario == "linger": # exec.completed, then the CLI never exits on its own + signal.signal(signal.SIGTERM, signal.SIG_IGN) + emit("exec.completed", result=result("succeeded", output="ok")) + emit("after.completed") time.sleep(60) sys.exit(0) diff --git a/packages/code-sdk-python/tests/test_cli.py b/packages/code-sdk-python/tests/test_cli.py new file mode 100644 index 000000000..71afc11a6 --- /dev/null +++ b/packages/code-sdk-python/tests/test_cli.py @@ -0,0 +1,115 @@ +"""Windows launcher resolution. Runs on every platform with synthetic launchers.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from minimax_code import MCodeNotFoundError +from minimax_code._cli import resolve_cli + +NPM_SHIM = """@ECHO off +GOTO start +:find_dp0 +SET dp0=%~dp0 +EXIT /b +:start +SETLOCAL +CALL :find_dp0 + +IF EXIST "%dp0%\\node.exe" ( + SET "_prog=%dp0%\\node.exe" +) ELSE ( + SET "_prog=node" + SET PATHEXT=%PATHEXT:;.JS;=;% +) + +endLocal & goto #_undefined_# 2>NUL || title %COMSPEC% & \ +"%_prog%" "%dp0%\\node_modules\\@minimax-ai\\code\\dist\\cli.js" %* +""" + +ROOT_LAUNCHER = ( + '@ECHO off\r\nSETLOCAL\r\nSET /P MCODE_RELEASE=<"%~dp0current"\r\n' + 'ECHO(%MCODE_RELEASE%| %SystemRoot%\\System32\\findstr.exe /R /X "[0-9A-Za-z][0-9A-Za-z._-]*" ' + ">NUL || EXIT /B 1\r\n" + 'CALL "%~dp0releases\\%MCODE_RELEASE%\\.mcode-launcher.cmd" %*\r\nEXIT /B %ERRORLEVEL%\r\n' +) + + +def touch(path: Path, text: str = "") -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + return path + + +def npm_install(root: Path, *, adjacent_node: bool) -> tuple[Path, Path]: + shim = touch(root / "mcode.cmd", NPM_SHIM) + entry = touch(root / "node_modules" / "@minimax-ai" / "code" / "dist" / "cli.js") + if adjacent_node: + touch(root / "node.exe") + return shim, entry + + +def installer_release(root: Path, release: str) -> tuple[Path, Path]: + node = touch(root / "runtime" / release / "node.exe") + entry = touch(root / "releases" / release / "dist" / "cli.js") + touch( + root / "releases" / release / ".mcode-launcher.cmd", + f'@ECHO off\r\n"{node}" "%~dp0dist\\cli.js" %*\r\nEXIT /B %ERRORLEVEL%\r\n', + ) + return node, entry + + +def test_npm_cmd_shim_runs_its_js_entry_with_path_node(tmp_path: Path) -> None: + shim, entry = npm_install(tmp_path, adjacent_node=False) + cli = resolve_cli(str(shim), {}, platform="win32") + assert cli.path == str(entry) and cli.node is None + assert cli.command(["--version"])[1:] == [str(entry), "--version"] + + +def test_npm_cmd_shim_prefers_adjacent_node(tmp_path: Path) -> None: + shim, entry = npm_install(tmp_path, adjacent_node=True) + cli = resolve_cli(str(shim), {}, platform="win32") + assert cli.command(["exec"]) == [str(tmp_path / "node.exe"), str(entry), "exec"] + + +def test_installer_launcher_follows_the_current_release_on_every_run(tmp_path: Path) -> None: + launcher = touch(tmp_path / "mcode.cmd", ROOT_LAUNCHER) + node_a, entry_a = installer_release(tmp_path, "0.7.4") + node_b, entry_b = installer_release(tmp_path, "0.7.5") + touch(tmp_path / "current", "0.7.4\r\n") + cli = resolve_cli(str(launcher), {}, platform="win32") + assert cli.command(["exec"]) == [str(node_a), str(entry_a), "exec"] + touch(tmp_path / "current", "0.7.5") # CLI self-update between Turns + assert cli.command(["exec"]) == [str(node_b), str(entry_b), "exec"] + touch(tmp_path / "current", "..\\evil") + with pytest.raises(MCodeNotFoundError, match="no longer points"): + cli.command(["exec"]) + + +def test_unknown_batch_file_is_rejected_instead_of_run_through_cmd(tmp_path: Path) -> None: + launcher = touch(tmp_path / "mcode.bat", "@echo off\r\nsomething-else %*\r\n") + with pytest.raises(MCodeNotFoundError, match="JS entry"): + resolve_cli(str(launcher), {}, platform="win32") + + +def test_windows_shell_shim_and_native_executable(tmp_path: Path) -> None: + entry = touch(tmp_path / "node_modules" / "@minimax-ai" / "code" / "dist" / "cli.js") + shim = touch( + tmp_path / "mcode", + '#!/bin/sh\nexec node "$basedir/node_modules/@minimax-ai/code/dist/cli.js" "$@"\n', + ) + assert resolve_cli(str(shim), {}, platform="win32").path == str(entry) + native = touch(tmp_path / "mcode.exe") + assert resolve_cli(str(native), {}, platform="win32").command(["x"]) == [str(native), "x"] + + +def test_windows_environment_names_are_case_insensitive(tmp_path: Path) -> None: + shim, entry = npm_install(tmp_path, adjacent_node=False) + assert resolve_cli(None, {"Mcode_Bin": str(shim)}, platform="win32").path == str(entry) + + +def test_posix_launchers_run_directly(tmp_path: Path) -> None: + launcher = touch(tmp_path / "mcode.cmd", NPM_SHIM) + assert resolve_cli(str(launcher), {}, platform="linux").command(["x"]) == [str(launcher), "x"] diff --git a/packages/code-sdk-python/tests/test_release_check.py b/packages/code-sdk-python/tests/test_release_check.py index d290f1507..eb1fa5ba5 100644 --- a/packages/code-sdk-python/tests/test_release_check.py +++ b/packages/code-sdk-python/tests/test_release_check.py @@ -25,11 +25,14 @@ def project( ) -> Path: (tmp_path / "src" / "minimax_code").mkdir(parents=True) (tmp_path / "pyproject.toml").write_text( - f'[build-system]\nrequires = []\n\n[project]\nname = "x"\nversion = "{version}"\n' + f'[build-system]\nrequires = []\n\n[project]\nname = "x"\nversion = "{version}"\n', + encoding="utf-8", + ) + (tmp_path / "src" / "minimax_code" / "__init__.py").write_text( + f'__version__ = "{init}"\n', encoding="utf-8" ) - (tmp_path / "src" / "minimax_code" / "__init__.py").write_text(f'__version__ = "{init}"\n') (tmp_path / "CHANGELOG.md").write_text( - f"# Changelog\n\n## {log} · 2026-01-02\n\n## 0.0.1 · 2025-01-01\n" + f"# Changelog\n\n## {log} · 2026-01-02\n\n## 0.0.1 · 2025-01-01\n", encoding="utf-8" ) return tmp_path diff --git a/packages/code-sdk-python/tests/test_sdk.py b/packages/code-sdk-python/tests/test_sdk.py index 36054f004..6395bba62 100644 --- a/packages/code-sdk-python/tests/test_sdk.py +++ b/packages/code-sdk-python/tests/test_sdk.py @@ -2,12 +2,16 @@ import asyncio import json +import os import pickle +import signal +import time from pathlib import Path from typing import Any import pytest +import minimax_code.thread as thread_module from minimax_code import ( AgentMessageItem, MCode, @@ -28,7 +32,7 @@ def calls(env: dict[str, str]) -> list[dict[str, Any]]: - text = Path(env["FAKE_MCODE_RECORD"]).read_text() + text = Path(env["FAKE_MCODE_RECORD"]).read_text(encoding="utf-8") return [json.loads(line) for line in text.splitlines()] @@ -278,3 +282,84 @@ def test_sync_client(fake_env: dict[str, str]) -> None: types = [e["type"] for e in thread.run_streamed("b")] assert types[-1] == "exec.completed" assert thread.id == "sess-new" + + +async def test_cancel_uses_sigterm_after_terminal_sigint( + fake_env: dict[str, str], tmp_path: Path +) -> None: + # Ctrl+C in a terminal delivers SIGINT to the CLI too. The SDK's own cancel must not send a + # second SIGINT, which would hit the CLI's default action and skip exec.completed. + log, pid_file = tmp_path / "cli.log", tmp_path / "cli.pid" + fake_env.update( + FAKE_MCODE_SCENARIO="slow", + FAKE_MCODE_LOG=str(log), + FAKE_MCODE_PID=str(pid_file), + FAKE_MCODE_SHUTDOWN_S="0.5", + ) + thread = MCode(MCodeOptions(env=fake_env)).start_thread() + streamed = await thread.run_streamed("x") + events = streamed.__aiter__() + await events.__anext__() + os.kill(int(pid_file.read_text(encoding="utf-8")), signal.SIGINT) + await asyncio.sleep(0.1) + await streamed.aclose() + assert log.read_text(encoding="utf-8").splitlines() == [ + "signal SIGINT", + "signal SIGTERM", + "completed", + ] + + +def test_cancel_timings_cover_the_cli_shutdown_budget() -> None: + assert thread_module.CANCEL_GRACE_S >= thread_module.CLI_SHUTDOWN_BUDGET_S == 60 + assert thread_module.BACKSTOP_EXTRA_S >= 90 + + +async def test_completed_turn_does_not_wait_for_grandchild_holding_stdout( + fake_env: dict[str, str], +) -> None: + fake_env["FAKE_MCODE_SCENARIO"] = "grandchild" + started = time.monotonic() + turn = await MCode(MCodeOptions(env=fake_env)).start_thread().run("x") + assert turn.final_response == "ok" + assert time.monotonic() - started < 5 + + +async def test_exit_without_result_does_not_wait_for_grandchild( + fake_env: dict[str, str], monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(thread_module, "EXIT_DRAIN_S", 0.2) + fake_env["FAKE_MCODE_SCENARIO"] = "grandchild_crash" + started = time.monotonic() + with pytest.raises(MCodeRunError): + await MCode(MCodeOptions(env=fake_env)).start_thread().run("x") + assert time.monotonic() - started < 5 + + +async def test_cli_lingering_after_exec_completed_is_stopped( + fake_env: dict[str, str], monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.setattr(thread_module, "TERMINATE_GRACE_S", 0.3) + fake_env["FAKE_MCODE_SCENARIO"] = "linger" + thread = MCode(MCodeOptions(env=fake_env)).start_thread() + started = time.monotonic() + assert (await thread.run("x")).final_response == "ok" + types = [event["type"] async for event in await thread.run_streamed("y")] + assert types[-1] == "exec.completed" and "after.completed" not in types + assert time.monotonic() - started < 5 + + +async def test_thread_cwd_is_pinned_when_the_thread_is_created( + fake_env: dict[str, str], tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + first, second = tmp_path / "first", tmp_path / "second" + (first / "sub").mkdir(parents=True) + second.mkdir() + client = MCode(MCodeOptions(env=fake_env)) + monkeypatch.chdir(first) + default = client.start_thread() + relative = client.start_thread(ThreadOptions(cwd="sub")) + monkeypatch.chdir(second) + await default.run("a") + await relative.run("b") + assert [c["args"][4] for c in calls(fake_env)] == [f"--cwd={first}", f"--cwd={first / 'sub'}"] diff --git a/release/public-source.json b/release/public-source.json index 2626c2a2a..ef279d6aa 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -536,6 +536,7 @@ "packages/code-sdk-python/src/minimax_code/types.py", "packages/code-sdk-python/tests/conftest.py", "packages/code-sdk-python/tests/fake_mcode.py", + "packages/code-sdk-python/tests/test_cli.py", "packages/code-sdk-python/tests/test_e2e.py", "packages/code-sdk-python/tests/test_release_check.py", "packages/code-sdk-python/tests/test_sdk.py",