Skip to content

fix(controller): fail fast for local runner on native Windows - #583

Merged
Zhiyuan He (hzy46) merged 1 commit into
microsoft:mainfrom
rioyu123:codex/fail-fast-windows-local-reconciler
Sep 2, 2026
Merged

Zhiyuan He (hzy46) merged 1 commit into
microsoft:mainfrom
rioyu123:codex/fail-fast-windows-local-reconciler

Conversation

@rioyu123

@rioyu123 Rio Yu (rioyu123) commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor

Closes #578

Summary

  • LocalReconciler.run() now raises a clear unsupported-platform RuntimeError on native Windows before it queries rollouts, spawns a worker, or enters shutdown.
  • Add regression tests for the Windows rejection path and for the supported-platform path.
  • Document the constraint in the controller configuration reference and the quick start, and point Windows users to WSL.

Problem

The local runner relies on POSIX process-group semantics: workers are spawned with start_new_session=True and cleaned up with os.killpg(). On native Windows os.killpg does not exist, so a timed-out rollout or a controller shutdown raised AttributeError from the cleanup path after the worker had already been spawned, and the rollout was never patched to its intended failed state.

Per the discussion in #578, native Windows is not a maintained platform for the local runner. This PR therefore does not attempt Windows process-tree cleanup. It makes the unsupported configuration fail at startup with an actionable message instead of failing later inside cleanup.

What changed

  • agentlightning/controller/local_reconciler.py: add _is_native_windows() (os.name == 'nt', so WSL is unaffected) and check it at the top of run(). Construction, package import, and runner_type=k8s are unchanged.
  • tests/controller/test_local_reconciler_platform.py: on native Windows, run() raises and neither _reconcile_loop, _shutdown, nor any client get/patch call is awaited; on a supported platform, run() still reconciles and shuts down.
  • docs/30-controller-configuration.md: add a platform-support note under Local runner limits.
  • docs/01-quick-start.md: add an early pointer to that note, since the quick start uses runner_type=local.

Not changed: the Operating System :: OS Independent classifier. The package remains importable and other runner types remain available on Windows; only the local subprocess runner is rejected. This PR does not claim Windows support for the local runner.

Verification

  • Native Windows 11, Python 3.12: python -m agentlightning.controller runner_type=local agl_server.url=http://127.0.0.1:9 exits with code 1 and prints RuntimeError: runner_type=local is not supported on native Windows; use Linux (for example, WSL) instead. No request reaches the server before the error.
  • uv run --locked --no-sync pytest -q tests/controller - 9 passed
  • uv run --locked --no-sync pytest -q tests --ignore=tests/verl - 66 passed (the tests/verl modules need the verl-cpu group, which is not installed locally; no VERL code is touched)
  • ruff check . and ruff format --check . - clean
  • pre-commit run --all-files and python scripts/check_headers.py - passed
  • pyright --pythonplatform Linux on the changed files - 0 errors. On a Windows host, pyright reports the pre-existing os.killpg / signal.SIGKILL stubs at the cleanup site; the same findings exist on main.
  • mkdocs build --strict - passed; the #local-runner-limits anchor resolves
  • uv build --no-sources - sdist and wheel built

Raise a clear unsupported-platform error from LocalReconciler.run()
on native Windows before any rollout is queried or a worker is spawned.
Add regression tests for both paths and document the constraint and
the WSL alternative.

Closes microsoft#578
Copilot AI balanced review requested due to automatic review settings September 2, 2026 04:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The focused implementation, tests, and documentation consistently satisfy the stated platform contract.

Pull request overview

Adds an early native-Windows guard for the POSIX-dependent local runner.

Changes:

  • Rejects local-runner startup on native Windows with actionable guidance.
  • Adds regression coverage for unsupported and supported platforms.
  • Documents the limitation and WSL alternative.
File summaries
File Description
agentlightning/controller/local_reconciler.py Adds the startup platform check.
tests/controller/test_local_reconciler_platform.py Tests rejection and normal execution paths.
docs/30-controller-configuration.md Documents local-runner platform support.
docs/01-quick-start.md Warns Windows quick-start users.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@hzy46
Zhiyuan He (hzy46) enabled auto-merge (squash) September 2, 2026 04:46
@hzy46
Zhiyuan He (hzy46) merged commit 218f1f7 into microsoft:main Sep 2, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LocalReconciler cleanup uses os.killpg on Windows

3 participants