Skip to content

Commit 01eee8a

Browse files
Kowserclaude
andcommitted
refactor(ai): remove Windows patch alias — gate calls apply_thread_isolation() directly
_patch_conductor_use_threads_on_windows was a private, single-caller delegating alias after the shim moved to worker_isolation; Windows and CONDUCTOR_WORKER_ISOLATION=thread now share one visible entry point. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0f189a9 commit 01eee8a

3 files changed

Lines changed: 24 additions & 27 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12-
- Worker isolation mode: `CONDUCTOR_WORKER_ISOLATION=thread` runs every worker as a thread instead of a `multiprocessing.Process` (default `process` is unchanged — `spawn` remains the start method). For environments where multiprocessing's fork+exec bootstraps (spawn children, `resource_tracker`) fail, e.g. Firecracker microVM guests. Thread-mode tradeoffs: no per-worker force-kill (shutdown is cooperative), CPU-bound workers share the GIL, and `signal.signal` becomes a no-op off the main thread. Implementation: the Windows-only Process→Thread shim moved to `conductor.client.automator.worker_isolation` (the old `worker_manager._patch_conductor_use_threads_on_windows` remains as a delegating alias) and now also swaps the logging-relay `Queue` for a plain `queue.Queue`
12+
- Worker isolation mode: `CONDUCTOR_WORKER_ISOLATION=thread` runs every worker as a thread instead of a `multiprocessing.Process` (default `process` is unchanged — `spawn` remains the start method). For environments where multiprocessing's fork+exec bootstraps (spawn children, `resource_tracker`) fail, e.g. Firecracker microVM guests. Thread-mode tradeoffs: no per-worker force-kill (shutdown is cooperative), CPU-bound workers share the GIL, and `signal.signal` becomes a no-op off the main thread. Implementation: the Windows-only Process→Thread shim moved to `conductor.client.automator.worker_isolation` (the private `worker_manager._patch_conductor_use_threads_on_windows` helper is removed — the Windows gate calls `apply_thread_isolation()` directly) and now also swaps the logging-relay `Queue` for a plain `queue.Queue`
1313

1414
- Canonical metrics mode: opt-in harmonized metric surface via `WORKER_CANONICAL_METRICS=true` -- [details](METRICS.md#detailed-technical-notes--unreleased)
1515
- `MetricsSettings` gains `clean_directory` and `clean_dead_pids` for opt-in stale `.db` file cleanup (both default to `False`)

src/conductor/ai/agents/runtime/worker_manager.py

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,17 +21,6 @@
2121
logger = logging.getLogger("conductor.ai.agents.worker_manager")
2222

2323

24-
def _patch_conductor_use_threads_on_windows() -> None:
25-
"""Deprecated alias — kept for existing callers.
26-
27-
The Process->Thread shim (plus the Queue->queue.Queue half it was
28-
missing) now lives in :mod:`conductor.client.automator.worker_isolation`
29-
and is also activated by ``CONDUCTOR_WORKER_ISOLATION=thread``, read by
30-
:class:`TaskHandler` itself. See :func:`apply_thread_isolation`.
31-
"""
32-
apply_thread_isolation()
33-
34-
3524
if TYPE_CHECKING:
3625
from conductor.client.automator.task_handler import TaskHandler
3726
from conductor.client.configuration.configuration import Configuration
@@ -89,8 +78,11 @@ def start(self) -> None:
8978
"""
9079
from conductor.client.automator.task_handler import TaskHandler
9180

81+
# Windows always needs thread isolation (spawn-pickling of worker
82+
# closures fails there); other environments opt in via
83+
# CONDUCTOR_WORKER_ISOLATION=thread, read by TaskHandler itself.
9284
if platform.system() == "Windows":
93-
_patch_conductor_use_threads_on_windows()
85+
apply_thread_isolation()
9486

9587
with self._lock:
9688
if self._task_handler is None:

tests/unit/ai/test_worker_manager.py

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -358,32 +358,37 @@ def test_filter_installed_on_conductor_logger(self):
358358
conductor_logger.removeFilter(f)
359359

360360

361-
class TestThreadPatchAlias:
362-
"""The Windows patch entry point now delegates to worker_isolation."""
363-
364-
def test_alias_delegates_to_apply_thread_isolation(self):
365-
import conductor.ai.agents.runtime.worker_manager as wm
361+
class TestWindowsThreadIsolation:
362+
"""On Windows the gate applies thread isolation via worker_isolation directly."""
366363

364+
def test_windows_gate_applies_thread_isolation(self):
365+
config = MagicMock()
366+
manager = WorkerManager(configuration=config)
367367
with patch(
368+
"conductor.ai.agents.runtime.worker_manager.platform.system",
369+
return_value="Windows",
370+
), patch(
368371
"conductor.ai.agents.runtime.worker_manager.apply_thread_isolation"
369-
) as mock_apply:
370-
wm._patch_conductor_use_threads_on_windows()
372+
) as mock_apply, patch(
373+
"conductor.client.automator.task_handler.TaskHandler"
374+
) as mock_th:
375+
mock_th.return_value.task_runner_processes = []
376+
mock_th.return_value.metrics_provider_process = None
377+
manager.start()
371378
mock_apply.assert_called_once_with()
372379

373-
def test_windows_gate_calls_the_alias(self):
374-
import conductor.ai.agents.runtime.worker_manager as wm
375-
380+
def test_non_windows_gate_does_not_apply(self):
376381
config = MagicMock()
377382
manager = WorkerManager(configuration=config)
378383
with patch(
379384
"conductor.ai.agents.runtime.worker_manager.platform.system",
380-
return_value="Windows",
385+
return_value="Linux",
381386
), patch(
382-
"conductor.ai.agents.runtime.worker_manager._patch_conductor_use_threads_on_windows"
383-
) as mock_patch, patch(
387+
"conductor.ai.agents.runtime.worker_manager.apply_thread_isolation"
388+
) as mock_apply, patch(
384389
"conductor.client.automator.task_handler.TaskHandler"
385390
) as mock_th:
386391
mock_th.return_value.task_runner_processes = []
387392
mock_th.return_value.metrics_provider_process = None
388393
manager.start()
389-
mock_patch.assert_called_once_with()
394+
mock_apply.assert_not_called()

0 commit comments

Comments
 (0)