refactor(app_controller): migrate _push_mma_state_update + _load_beads to Result helpers (Phase 7)
Tasks 7.4 + 7.5: Migrate two more strict-violation sites to proper Result[T] propagation: - _push_mma_state_update: legacy wrapper preserved (fire-and-forget semantics) but routes errors through _report_worker_error. New _push_mma_state_update_result helper returns Result[None]. - _load_active_tickets.beads inner: extracted to _load_beads_from_path_result helper; outer merges errors via _report_worker_error. Per Phase 7 spec 22.5.3 + 22.5.4: - Each helper catches OSError/IOError/ValueError/TypeError/KeyError/ AttributeError -> ErrorInfo(original=e). - Drain is Pattern 4 telemetry via _report_worker_error (Pattern 4 = in-process telemetry buffer that sub-track 4 forwards to GUI per error_handling.md:421). TIER-2 READ conductor/code_styleguides/error_handling.md end-to-end before this commit.
This commit is contained in:
+51
-18
@@ -5040,12 +5040,24 @@ class AppController:
|
||||
mutation (ticket status change, bulk execute, reorder, etc.) so
|
||||
the in-memory state (self.active_track.tickets) and the on-disk
|
||||
state match self.active_tickets.
|
||||
|
||||
DEPRECATED (Phase 7 Task 7.4): legacy fire-and-forget wrapper. New
|
||||
callers should use `_push_mma_state_update_result()` which returns
|
||||
Result[None] and propagates errors via `_report_worker_error`.
|
||||
[C: tests/test_gui_phase4.py:test_push_mma_state_update, tests/test_ticket_queue.py:TestBulkOperations, tests/test_ticket_queue.py:TestReorder]
|
||||
"""
|
||||
result = self._push_mma_state_update_result()
|
||||
if not result.ok:
|
||||
self._report_worker_error("push_mma_state_update", result)
|
||||
|
||||
def _push_mma_state_update_result(self) -> "Result[None]":
|
||||
"""Phase 7 Task 7.4: push MMA state with Result propagation (Phase 6 Group 6.5).
|
||||
On failure: OSError/IOError/ValueError/TypeError/KeyError/AttributeError -> ErrorInfo(original=e).
|
||||
Caller drains via `_report_worker_error` (Pattern 4 telemetry)."""
|
||||
try:
|
||||
from src import project_manager
|
||||
track = self.active_track
|
||||
if track is None: return
|
||||
if track is None: return OK
|
||||
new_tickets = [
|
||||
models.Ticket(
|
||||
id=t.get("id", ""),
|
||||
@@ -5059,9 +5071,14 @@ class AppController:
|
||||
track.tickets = new_tickets
|
||||
state = models.TrackState(metadata=track, tasks=list(new_tickets))
|
||||
project_manager.save_track_state(track.id, state, self.active_project_root)
|
||||
return OK
|
||||
except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError) as e:
|
||||
logging.getLogger(__name__).debug("push MMA state failed: %s", e, extra={"source": "app_controller._push_mma_state_update"})
|
||||
print(f"Error pushing MMA state: {e}", file=sys.stderr)
|
||||
return Result(data=None, errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=str(e),
|
||||
source="app_controller._push_mma_state_update_result",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
def _load_active_tickets(self) -> None:
|
||||
"""
|
||||
@@ -5076,21 +5093,37 @@ class AppController:
|
||||
if getattr(self, "ui_project_execution_mode", None) == "beads":
|
||||
base = getattr(self, "ui_files_base_dir", None) or getattr(self, "active_project_root", None)
|
||||
if base:
|
||||
try:
|
||||
from src import beads_client
|
||||
bclient = beads_client.BeadsClient(Path(base))
|
||||
if bclient.is_initialized():
|
||||
for bead in bclient.list_beads():
|
||||
self.active_tickets.append({
|
||||
"id": bead.id,
|
||||
"title": bead.title,
|
||||
"description": bead.description,
|
||||
"status": bead.status,
|
||||
"depends_on": [],
|
||||
})
|
||||
except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError) as e:
|
||||
logging.getLogger(__name__).debug("load beads failed: %s", e, extra={"source": "app_controller._load_active_tickets.beads"})
|
||||
print(f"Error loading beads: {e}")
|
||||
beads_result = self._load_beads_from_path_result(Path(base))
|
||||
if beads_result.ok:
|
||||
for bead in beads_result.data:
|
||||
self.active_tickets.append({
|
||||
"id": bead.id,
|
||||
"title": bead.title,
|
||||
"description": bead.description,
|
||||
"status": bead.status,
|
||||
"depends_on": [],
|
||||
})
|
||||
elif not beads_result.ok:
|
||||
self._report_worker_error("load_beads", beads_result)
|
||||
|
||||
def _load_beads_from_path_result(self, beads_path: "Path") -> "Result[List[Any]]":
|
||||
"""Phase 7 Task 7.5: load beads from a path with Result propagation (Phase 6 Group 6.5).
|
||||
On failure: OSError/IOError/ValueError/TypeError/KeyError/AttributeError -> ErrorInfo(original=e).
|
||||
Caller (`_load_active_tickets`) drains via `_report_worker_error`."""
|
||||
try:
|
||||
from src import beads_client
|
||||
bclient = beads_client.BeadsClient(beads_path)
|
||||
if not bclient.is_initialized():
|
||||
return Result(data=[])
|
||||
beads = list(bclient.list_beads())
|
||||
return Result(data=beads)
|
||||
except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError) as e:
|
||||
return Result(data=[], errors=[ErrorInfo(
|
||||
kind=ErrorKind.INTERNAL,
|
||||
message=str(e),
|
||||
source=f"app_controller._load_beads_from_path_result[{beads_path}]",
|
||||
original=e,
|
||||
)])
|
||||
|
||||
#region: --- Config I/O (single source of truth) ---
|
||||
def load_config(self) -> Dict[str, Any]:
|
||||
|
||||
@@ -575,17 +575,21 @@ def test_push_mma_state_update_records_error_in_state():
|
||||
|
||||
def test_load_beads_from_path_returns_result():
|
||||
"""
|
||||
Phase 7 Task 7.5: _load_beads_from_path_result returns Result[List[Ticket]].
|
||||
Phase 7 Task 7.5: _load_beads_from_path_result returns Result[List[Bead]].
|
||||
On error: ErrorInfo(original=e).
|
||||
"""
|
||||
from pathlib import Path
|
||||
from src.app_controller import AppController
|
||||
from unittest.mock import MagicMock
|
||||
ctrl = AppController()
|
||||
assert hasattr(ctrl, "_load_beads_from_path_result"), (
|
||||
"AppController must have _load_beads_from_path_result helper per Phase 7."
|
||||
)
|
||||
# Success path returns Result with empty list when no tickets
|
||||
with patch("src.app_controller.beads_client.list_tickets", return_value=[]):
|
||||
result = ctrl._load_beads_from_path_result("/tmp/fake_beads_path")
|
||||
# Success path returns Result with empty list when not initialized
|
||||
fake_bclient = MagicMock()
|
||||
fake_bclient.is_initialized.return_value = False
|
||||
with patch("src.beads_client.BeadsClient", return_value=fake_bclient):
|
||||
result = ctrl._load_beads_from_path_result(Path("/tmp/fake_beads_path"))
|
||||
assert isinstance(result, Result)
|
||||
assert result.ok is True
|
||||
assert result.data == []
|
||||
@@ -593,14 +597,16 @@ def test_load_beads_from_path_returns_result():
|
||||
|
||||
def test_load_beads_from_path_records_error_on_failure():
|
||||
"""
|
||||
Phase 7 Task 7.5: when beads_client.list_tickets raises, the helper returns
|
||||
Phase 7 Task 7.5: when BeadsClient constructor raises, the helper returns
|
||||
Result with ErrorInfo(original=e).
|
||||
"""
|
||||
from pathlib import Path
|
||||
from src.app_controller import AppController
|
||||
from unittest.mock import MagicMock
|
||||
ctrl = AppController()
|
||||
with patch("src.app_controller.beads_client.list_tickets",
|
||||
with patch("src.beads_client.BeadsClient",
|
||||
side_effect=OSError("beads path not found")):
|
||||
result = ctrl._load_beads_from_path_result("/tmp/nonexistent")
|
||||
result = ctrl._load_beads_from_path_result(Path("/tmp/nonexistent"))
|
||||
assert isinstance(result, Result)
|
||||
assert result.ok is False
|
||||
assert len(result.errors) == 1
|
||||
|
||||
Reference in New Issue
Block a user