ed
7bcb5a8c07
Eliminates 22 call sites that bypassed the AppController state owner
and read/wrote config.toml directly. AppController is now the single
source of truth for self.config; gui_2.py, commands.py, etc. go
through controller.save_config() / controller.load_config().
Production changes:
- src/models.py: rename load_config -> _load_config_from_disk,
save_config -> _save_config_to_disk (private I/O primitives)
- src/app_controller.py: add public load_config()/save_config() methods
that own the state. Update 3 internal call sites and 3 ConductorEngine
call sites to pass max_workers from self.config
- src/multi_agent_conductor.py: ConductorEngine.__init__ now takes
max_workers as a parameter (caller responsibility, not I/O primitive)
- src/external_editor.py: get_default_launcher() takes config as a
parameter; gui_2.py:1311,4776 pass app.config
- src/gui_2.py: 17 sites of models.save_config(X.config) replaced with
X.save_config() (delegates via __getattr__ to controller)
- src/commands.py: save_all() uses app.save_config()
Test changes (route through controller, not I/O primitive):
- tests/conftest.py: mock_app and app_instance fixtures now patch
AppController.load_config/save_config instead of models I/O primitives
- 18 other test files: patches renamed from models._save_config_to_disk
to AppController.save_config (and same for load_config)
- tests/test_app_controller_mcp.py: use SLOP_CONFIG env var instead of
patching removed CONFIG_PATH module constant
- tests/test_parallel_execution.py: pass max_workers=2 explicitly to
ConductorEngine (caller no longer reads config)
- tests/test_gui_paths.py: add save_config=MagicMock() to MockApp;
assert on controller method, not I/O primitive
- tests/test_models_no_top_level_tomli_w.py: still calls private
_save_config_to_disk directly (the only allowed exception; tests
the lazy-load behavior of the primitive itself)
New files:
- scripts/audit_no_models_config_io.py: enforces the rule (--strict,
--json modes; AST-based docstring detection to avoid false positives)
- conductor/code_styleguides/config_state_owner.md: documents the rule
Verification:
- 67 targeted tests pass
- scripts/audit_no_models_config_io.py --strict returns 0
This is the architectural cleanup that surfaced during the
audit_architectural_cheats_20260607 review. Closes the smoke-gun
CONFIG_PATH module constant (already done in 0c7ebf22) AND the
free-function models.load_config/save_config smell.
[conductor(checkpoint): config-iO-refactor-20260607]
113 lines
3.2 KiB
Python
113 lines
3.2 KiB
Python
import threading
|
|
import time
|
|
import sys
|
|
import pytest
|
|
from unittest.mock import MagicMock
|
|
from src.multi_agent_conductor import WorkerPool
|
|
|
|
def test_worker_pool_limit():
|
|
max_workers = 2
|
|
pool = WorkerPool(max_workers=max_workers)
|
|
|
|
def slow_task(event):
|
|
event.set()
|
|
time.sleep(0.5)
|
|
|
|
event1 = threading.Event()
|
|
event2 = threading.Event()
|
|
event3 = threading.Event()
|
|
|
|
# Spawn 2 tasks
|
|
t1 = pool.spawn("t1", slow_task, (event1,))
|
|
t2 = pool.spawn("t2", slow_task, (event2,))
|
|
|
|
assert t1 is not None
|
|
assert t2 is not None
|
|
assert pool.get_active_count() == 2
|
|
assert pool.is_full() is True
|
|
|
|
# Try to spawn a 3rd task
|
|
t3 = pool.spawn("t3", slow_task, (event3,))
|
|
assert t3 is None
|
|
assert pool.get_active_count() == 2
|
|
|
|
# Wait for tasks to finish
|
|
event1.wait()
|
|
event2.wait()
|
|
pool.join_all()
|
|
|
|
assert pool.get_active_count() == 0
|
|
assert pool.is_full() is False
|
|
|
|
def test_worker_pool_tracking():
|
|
pool = WorkerPool(max_workers=4)
|
|
|
|
def task(ticket_id):
|
|
time.sleep(0.1)
|
|
|
|
pool.spawn("ticket_1", task, ("ticket_1",))
|
|
pool.spawn("ticket_2", task, ("ticket_2",))
|
|
|
|
assert "ticket_1" in pool._active
|
|
assert "ticket_2" in pool._active
|
|
|
|
pool.join_all()
|
|
assert len(pool._active) == 0
|
|
|
|
def test_worker_pool_completion_cleanup():
|
|
pool = WorkerPool(max_workers=4)
|
|
|
|
def fast_task():
|
|
pass
|
|
|
|
pool.spawn("t1", fast_task, ())
|
|
time.sleep(0.2) # Give it time to finish and run finally block
|
|
|
|
assert pool.get_active_count() == 0
|
|
assert "t1" not in pool._active
|
|
|
|
from unittest.mock import patch
|
|
from src.models import Track, Ticket
|
|
from src.multi_agent_conductor import ConductorEngine
|
|
|
|
@patch('src.multi_agent_conductor.run_worker_lifecycle')
|
|
def test_conductor_engine_pool_integration(mock_lifecycle):
|
|
# Create 4 independent tickets
|
|
tickets = [
|
|
Ticket(id=f"t{i}", description=f"task {i}", status="todo")
|
|
for i in range(4)
|
|
]
|
|
track = Track(id="test_track", description="test", tickets=tickets)
|
|
|
|
# Set up engine with auto_queue and explicit max_workers=2.
|
|
# ConductorEngine no longer reads config itself; the caller passes max_workers.
|
|
engine = ConductorEngine(track, auto_queue=True, max_workers=2)
|
|
sys.stderr.write(f"[TEST] engine.pool.max_workers = {engine.pool.max_workers}\n")
|
|
assert engine.pool.max_workers == 2
|
|
|
|
# Slow down lifecycle to capture parallel state
|
|
def slow_lifecycle(ticket, *args, **kwargs):
|
|
# Set to in_progress immediately to simulate the status change
|
|
# (The engine usually does this, but we want to be sure)
|
|
time.sleep(0.5)
|
|
ticket.status = "completed"
|
|
|
|
mock_lifecycle.side_effect = slow_lifecycle
|
|
|
|
# Run exactly 1 tick
|
|
engine.run(max_ticks=1)
|
|
|
|
# Verify only 2 were marked in_progress/spawned
|
|
# Because we only ran for one tick, and there were 4 ready tasks,
|
|
# it should have tried to spawn as many as possible (limit 2).
|
|
in_progress = [tk for tk in tickets if tk.status == "in_progress"]
|
|
# Also count those that already finished if the sleep was too short
|
|
completed = [tk for tk in tickets if tk.status == "completed"]
|
|
|
|
sys.stderr.write(f"[TEST] in_progress={len(in_progress)} completed={len(completed)}\n")
|
|
assert len(in_progress) + len(completed) == 2
|
|
assert engine.pool.get_active_count() <= 2
|
|
|
|
# Cleanup: wait for mock threads to finish or join_all
|
|
engine.pool.join_all()
|