From 9cfbb980bd214ffc4377b4fd0372885b0f31e535 Mon Sep 17 00:00:00 2001 From: Ed_ Date: Thu, 2 Jul 2026 10:23:14 -0400 Subject: [PATCH] fix(mma_lifecycle): load + start + active_tickets sync for batched tests The test_visual_sim_mma_v2 failure in tier-3 batch context was caused by state pollution from prior live_gui tests sharing the subprocess: 1. track state file missing for leftover track: `_cb_load_track_result` accessed `state.metadata.id` and `state.metadata.name`, but `EMPTY_TRACK_STATE` (returned when no state.toml exists for a track_id) had `metadata={}` (a dict, not a TrackMetadata object). That raised `'dict' object has no attribute 'id'`. Fixed by normalizing metadata: dict -> TrackMetadata.from_dict, TrackMetadata stays, anything else -> TrackMetadata(id=track_id, name=track_id). 2. active_track and active_tickets never reached the App: `_cb_load_track_result` set `self.active_track` (controller) and `self.active_tickets = []` (via `_load_active_tickets`) but never mirrored to `self._app.active_track` / `self._app.active_tickets`. The /api/gui/mma_status endpoint reads `app.X` first via `_get_app_attr`, so it returned None / [] and the test's poll `at_id == track_id and bool(s.get('active_tickets'))` failed. Fixed by mirroring active_track / active_tickets / active_tier to the App. 3. leftover tracks list in batched run: Without btn_reset, `app.tracks` (App-side) accumulates tracks from earlier tests in the session. `_get_app_attr(app, 'tracks', [])` then returns stale leftovers, and the test's `target_track = next((... if 'hello_world' in t.get('title') else tracks_list[0]))` picks a leftover with no on-disk state file. Fixed in TWO places: (a) btn_reset now also clears `app.tracks = []` so each test starts clean if it calls btn_reset. (b) tests/test_visual_sim_mma_v2.py now calls `client.click('btn_reset')` at the start. The test was the only one in its tier that did NOT reset; with the live_gui subprocess shared across batched tests, that's the source of the state pollution. Also reverted the `TrackState.metadata` default change (dict -> TrackMetadata) because it broke TrackState() construction (TrackMetadata requires `id`/`name`). The metadata normalization in `_cb_load_track_result` is sufficient and preserves backward compatibility with on-disk state.toml files. Verified: tests/test_visual_sim_mma_v2.py passes in isolation (58.36s) and tier-3 batch passes when run standalone. Other tests in tier-3 have pre-existing render-loop contention flakes in batched xdist mode that are unrelated to this fix. --- src/app_controller.py | 46 ++++++++++++++++++++++++++++++--- tests/test_visual_sim_mma_v2.py | 13 ++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/src/app_controller.py b/src/app_controller.py index efdf953d..e4bb61f2 100644 --- a/src/app_controller.py +++ b/src/app_controller.py @@ -3983,6 +3983,14 @@ class AppController: if hasattr(app, 'active_track'): app.active_track = None if hasattr(app, 'proposed_tracks'): app.proposed_tracks = [] if hasattr(app, 'mma_streams'): app.mma_streams.clear() + # Clear App-side tracks list too. The live_gui subprocess is shared + # across many live_gui tests in batched xdist mode; without this, + # a test that runs after `test_mma_concurrent_tracks_sim` (which + # creates Track A and Track B) would see those leftovers in + # `tracks_list[0]` and load a track whose on-disk state file may not + # exist, producing `'dict' object has no attribute 'id'` / + # `bool(active_tickets)` failures (test_visual_sim_mma_v2 Stage 6). + if hasattr(app, 'tracks'): app.tracks = [] def _handle_compress_discussion(self) -> None: def worker() -> "Result[None]": @@ -5089,11 +5097,43 @@ class AppController: tickets.append(Ticket(**t)) else: tickets.append(t) + # Defensive: state.metadata may be a dict (from EMPTY_TRACK_STATE or a + # legacy on-disk state.toml where metadata was serialized as a raw dict). + # TrackMetadata.from_dict accepts either, but `state.metadata.id` / + # `state.metadata.name` only work on TrackMetadata instances. Normalize + # to TrackMetadata first. If the state has no real metadata (e.g. + # EMPTY_TRACK_STATE returned because the on-disk state.toml is missing + # for a track that's still listed in `self.tracks`), fall back to + # the track_id passed to the click handler so `active_track.id` + # matches what the test/UI was asking for. + meta = state.metadata + if not isinstance(meta, TrackMetadata): + if isinstance(meta, dict): + meta = TrackMetadata.from_dict(meta) + else: + meta = TrackMetadata(id=track_id, name=track_id) + track_id_final = meta.id or track_id + track_name_final = meta.name or meta.id or track_id self.active_track = Track( - id=state.metadata.id, - description=state.metadata.name, + id=track_id_final, + description=track_name_final, tickets=tickets ) + # Sync App-side state too. Without this, /api/gui/mma_status (which + # reads `app.active_tickets` via `_get_app_attr`) returns the stale + # App-side list and `bool(s.get('active_tickets'))` is False, breaking + # any test that polls for active_tickets after load (e.g. + # test_visual_sim_mma_v2 Stage 6). The controller's `self.active_tickets` + # is cleared by `_load_active_tickets` (see below) and the test wants + # the freshly-loaded track's tickets visible in `app.active_tickets`. + # Also sync `app.active_track` and `app.active_tier` so the endpoint + # (which checks App first via `_get_app_attr`) returns the freshly- + # loaded track, not a stale None or the previous test's track. + if hasattr(self, '_app') and self._app is not None: + self._app.active_track = self.active_track + if tickets: + self._app.active_tickets = tickets + self._app.active_tier = f"Tier 2 (Tech Lead): {track_name_final}" # Keep dicts for UI table self._load_active_tickets() # Load track-scoped history @@ -5104,7 +5144,7 @@ class AppController: else: self.disc_entries.clear() self._recalculate_session_usage() - self.ai_status = f"Loaded track: {state.metadata.name}" + self.ai_status = f"Loaded track: {track_name_final}" return OK except (OSError, IOError, ValueError, TypeError, KeyError, AttributeError, tomllib.TOMLDecodeError) as e: return Result(data=None, errors=[ErrorInfo( diff --git a/tests/test_visual_sim_mma_v2.py b/tests/test_visual_sim_mma_v2.py index 66b109d3..a843acc5 100644 --- a/tests/test_visual_sim_mma_v2.py +++ b/tests/test_visual_sim_mma_v2.py @@ -64,6 +64,19 @@ def test_mma_complete_lifecycle(live_gui) -> None: client = api_hook_client.ApiHookClient() assert client.wait_for_server(timeout=15), "Hook server did not start" + # Clear any stale state from prior live_gui tests in this batched + # subprocess. Without btn_reset, leftover tracks (created by earlier + # tests like test_mma_concurrent_tracks_sim / test_visual_mma) sit in + # `app.tracks` and become `tracks_list[0]`. The test's `target_track = + # next(..., tracks_list[0])` then loads a leftover track whose on-disk + # state file does not exist, so `_cb_load_track_result` falls back to + # EMPTY_TRACK_STATE → `state.tasks == []` → `bool(active_tickets)` is + # False → Stage 6 polls fail. btn_reset clears `app.tracks` / `app.active_tickets` + # / `app.active_track` so `tracks_list` reflects only tracks accepted + # in this test's plan_epic + accept_tracks batch. + client.click("btn_reset") + time.sleep(2) + # ------------------------------------------------------------------ # Stage 1: Provider setup # ------------------------------------------------------------------