Skip to content

fix(sdk-python): stop the CLI gracefully, bound pipe waits, pin cwd, resolve Windows launchers - #586

Merged
hetaoBackend merged 2 commits into
mainfrom
fix/python-sdk-process-lifecycle
Nov 4, 2026
Merged

hetaoBackend merged 2 commits into
mainfrom
fix/python-sdk-process-lifecycle

Conversation

@1anZhang

@1anZhang 1anZhang commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #585 (base fix/python-sdk-4-bugs); GitHub retargets it to main once #585 merges. Review the diff against #585 only.

Change

Process-lifecycle and Windows fixes for the Python SDK (packages/code-sdk-python) found while reviewing #585 before 0.1.0. All reproduced against the published 0.1.0b1 and still present with #585 applied. Aligns the SDK with the TypeScript SDK's exec-process.ts / project-bin.ts.

  1. Ctrl+C killed the CLI before exec.completed. The CLI registers one-shot (once) SIGINT/SIGTERM handlers. A terminal Ctrl+C delivers SIGINT to the CLI's process group; the SDK's cancel then sent a second SIGINT, which hit the default action. Repro: CLI killed after 0.7 s without completing. Fix: cancel with SIGTERM (as the TS SDK does). After: the CLI finishes its graceful shutdown and writes exec.completed.
  2. Cancel grace (10 s) and backstop (timeout_ms + 30 s) were shorter than the CLI's 60 s shutdown budget. A CLI still shutting down cleanly was SIGKILLed (repro: 20 s shutdown killed at 10 s). Fix: CANCEL_GRACE_S = 60, BACKSTOP_EXTRA_S = 60 + 30, matching CLI_SHUTDOWN_BUDGET_MS / CLI_STARTUP_SLACK_MS.
  3. A grandchild holding stdout/stderr kept thread.run() waiting until it exited (repro: 25.5 s after exec.completed). asyncio's Process.wait() only resolves after every pipe closes, so exit is now detected from returncode. exec.completed is terminal (later output ignored, a lingering CLI is killed after 5 s), and after exit the remaining output is drained for at most 2 s (TS: terminateGraceMs, exitDrainMs). After: 2.6 s.
  4. cwd was resolved per Turn, so an os.chdir() between Turns moved an existing Session to another workspace, which the CLI rejects. A Thread now pins its absolute cwd at creation.
  5. Windows ran mcode.cmd through cmd.exe. Both the official installer and npm install a .cmd launcher; arguments were reinterpreted by cmd.exe and cancel (terminate()) stopped only cmd.exe, orphaning the CLI. The SDK now follows npm/pnpm shims and the installer's versioned launcher to the JS entry plus its Node runtime (adjacent node.exe for npm, the pinned runtime for installer releases; the release pointer is re-read before every Turn), rejects unrecognized batch files, and reads MCODE_BIN/PATH case-insensitively on Windows.

Also: CI matrix adds Python 3.14 and a windows-latest job (resolver and option tests; fake-CLI process tests are POSIX-only and skip there); Python version classifiers; README and CHANGELOG (## Unreleased) updated. Version unchanged.

  • PR labels: bug.

Validation

  • uv run ruff check, ruff format --check, mypy (strict), pytest -W error: 57 passed, 3 skipped (opt-in e2e) on Python 3.10, 3.13 and 3.14 (macOS arm64).
  • New tests: SIGTERM after a terminal SIGINT completes gracefully; timings cover the shutdown budget; grandchild holding pipes with and without a result; lingering CLI after exec.completed; cwd pinned at creation; tests/test_cli.py resolves synthetic npm shims (with/without adjacent node.exe), the installer's versioned launcher across a self-update, an invalid release pointer, unknown batch files, shell shims, .exe, and case-insensitive env names.
  • Standalone fake-CLI reproductions for 1–3: before / after as listed above.
  • Real CLI (mcode 0.7.1): e2e 3 passed; breaking out of a streamed Turn and immediately running the next Turn on the same Thread succeeds after the graceful shutdown.
  • release_check.py metadata and dist pass on a fresh build; source-inventory.mjs --write recorded tests/test_cli.py.
  • NOT RUN: real Windows (launcher parsing is exercised with synthetic files on all platforms; the new windows-latest job runs it on Windows in CI); the monorepo pnpm verify locally (no TypeScript changes; CI runs it). Results are offline/fake-CLI except the e2e and cancel runs noted above.

Publication and contribution checks

  • I have permission to contribute these changes under the existing licenses applicable to the changed files/packages; imported material and its provenance are identified and existing notices are preserved.
  • No credentials, account data, real user content, internal source history or private review material is included.
  • Added/removed source files were reviewed before regenerating release/public-source.json; new tests are declared in test/vitest-suites.json where applicable (one pytest file added; collected by pytest, not Vitest).
  • Shared English/Chinese documentation and capability/verification records are updated where applicable. Mock/offline results are not described as live-service acceptance.

Maintainer handoff

Publication scope or license changes (if any): none.

Shared-source port: pending. Follow-up for the TypeScript SDK: it also resolves cwd per run (args.ts), so item 4 applies there too.


Autofix with [code]smith
Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.

@1anZhang 1anZhang added the bug Something isn't working label Nov 4, 2026
- Cancel with SIGTERM: after a terminal Ctrl+C already delivered SIGINT,
  a second SIGINT hit the CLI's default action and skipped exec.completed.
- Wait for the CLI's 60 s shutdown budget before SIGKILL and fire the
  timeout_ms backstop 90 s after timeout_ms, matching the TypeScript SDK.
- Treat exec.completed as terminal, kill a CLI lingering 5 s after it, and
  detect exit from returncode so a grandchild holding stdout or stderr no
  longer keeps the Turn open; drain output for at most 2 s after exit.
- Pin ThreadOptions.cwd when a Thread is created.
- On Windows, run the JS entry and Node runtime behind npm/pnpm shims and
  the installer's versioned launcher instead of the .cmd file.
- CI adds Python 3.14 and a windows-latest job.
… encodings

The new windows-latest job failed in test_release_check.py: its helper wrote
CHANGELOG.md with the locale code page (cp1252 on Windows), so the '·' in
the heading was not UTF-8. Pass encoding="utf-8" to every read_text and
write_text in the tests and run pytest with PYTHONWARNDEFAULTENCODING=1 so
implicit encodings fail on every platform.
@1anZhang
1anZhang force-pushed the fix/python-sdk-process-lifecycle branch from 1fe5546 to e9019de Compare November 4, 2026 01:41
@1anZhang
1anZhang changed the base branch from fix/python-sdk-4-bugs to main November 4, 2026 01:41
@1anZhang 1anZhang mentioned this pull request Nov 4, 2026
3 of 4 tasks
@hetaoBackend
hetaoBackend merged commit e8d30c1 into main Nov 4, 2026
17 checks passed
@hetaoBackend
hetaoBackend deleted the fix/python-sdk-process-lifecycle branch November 4, 2026 01:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants