fix(ui): repair startup crash in TeamView and tighten the headless guard
Launching the app died with "'dict' object has no attribute 'readiness'".
TeamView.__init__ forwarded its legacy app_state dict into update_data(snapshot),
whose guard only handled None, so {} reached snapshot.readiness. TeamView is the
default view, so the window never appeared.
Fixes the call site and hardens every snapshot guard to fall back on anything
that is not a HubSnapshot. Adds tests/test_view_startup_contract.py, which
checks both conditions statically and therefore runs headless; verified to fail
on the pre-fix sources.
The GUI-import invariant previously matched the literal string "customtkinter",
which missed modules pulling it transitively. Rewritten over the AST, it
immediately found test_codex_opencode_wizard.py importing router.ui.components
without pytest.importorskip — the same defect that took the release gate down
once before, hidden until now behind conftest's name-based skip.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
93883f9ce4
commit
a9c04b75a2
8 changed files with 118 additions and 10 deletions
|
|
@ -347,7 +347,9 @@ class AccountsView(ctk.CTkFrame):
|
||||||
|
|
||||||
def update_data(self, snapshot: Optional[HubSnapshot] = None):
|
def update_data(self, snapshot: Optional[HubSnapshot] = None):
|
||||||
"""Update views by reusing widgets and updating properties in place."""
|
"""Update views by reusing widgets and updating properties in place."""
|
||||||
if snapshot is None:
|
if not isinstance(snapshot, HubSnapshot):
|
||||||
|
# A non-snapshot (e.g. a legacy app_state dict) must fall back
|
||||||
|
# to the store rather than crash the view.
|
||||||
snapshot = HubStateStore.get().get_snapshot()
|
snapshot = HubStateStore.get().get_snapshot()
|
||||||
|
|
||||||
self._last_rendered_generation = snapshot.generation
|
self._last_rendered_generation = snapshot.generation
|
||||||
|
|
|
||||||
|
|
@ -46,8 +46,10 @@ class HealthView(ctk.CTkFrame):
|
||||||
for w in self.scroll.winfo_children():
|
for w in self.scroll.winfo_children():
|
||||||
w.destroy()
|
w.destroy()
|
||||||
|
|
||||||
from antigravity_provider.router.state_store import HubStateStore
|
from antigravity_provider.router.state_store import HubSnapshot, HubStateStore
|
||||||
if snapshot is None:
|
if not isinstance(snapshot, HubSnapshot):
|
||||||
|
# Callers have historically passed legacy app_state dicts here; a
|
||||||
|
# non-snapshot must fall back to the store, not crash the view.
|
||||||
snapshot = HubStateStore.get().get_snapshot()
|
snapshot = HubStateStore.get().get_snapshot()
|
||||||
|
|
||||||
readiness = snapshot.readiness
|
readiness = snapshot.readiness
|
||||||
|
|
|
||||||
|
|
@ -43,8 +43,10 @@ class ProvidersView(ctk.CTkFrame):
|
||||||
for w in self.scroll.winfo_children():
|
for w in self.scroll.winfo_children():
|
||||||
w.destroy()
|
w.destroy()
|
||||||
|
|
||||||
from antigravity_provider.router.state_store import HubStateStore
|
from antigravity_provider.router.state_store import HubSnapshot, HubStateStore
|
||||||
if snapshot is None:
|
if not isinstance(snapshot, HubSnapshot):
|
||||||
|
# Callers have historically passed legacy app_state dicts here; a
|
||||||
|
# non-snapshot must fall back to the store, not crash the view.
|
||||||
snapshot = HubStateStore.get().get_snapshot()
|
snapshot = HubStateStore.get().get_snapshot()
|
||||||
|
|
||||||
summaries = snapshot.providers
|
summaries = snapshot.providers
|
||||||
|
|
|
||||||
|
|
@ -137,7 +137,9 @@ class RoutingView(ctk.CTkFrame):
|
||||||
|
|
||||||
def update_data(self, snapshot: Optional[HubSnapshot] = None):
|
def update_data(self, snapshot: Optional[HubSnapshot] = None):
|
||||||
"""Update routing view using cached snapshot and reusable role widgets."""
|
"""Update routing view using cached snapshot and reusable role widgets."""
|
||||||
if snapshot is None:
|
if not isinstance(snapshot, HubSnapshot):
|
||||||
|
# A non-snapshot (e.g. a legacy app_state dict) must fall back
|
||||||
|
# to the store rather than crash the view.
|
||||||
snapshot = HubStateStore.get().get_snapshot()
|
snapshot = HubStateStore.get().get_snapshot()
|
||||||
|
|
||||||
self._last_rendered_generation = snapshot.generation
|
self._last_rendered_generation = snapshot.generation
|
||||||
|
|
|
||||||
|
|
@ -203,7 +203,7 @@ class TeamView(ctk.CTkFrame):
|
||||||
self.on_action = on_action
|
self.on_action = on_action
|
||||||
self._card_widgets: List[AgentCardWidget] = []
|
self._card_widgets: List[AgentCardWidget] = []
|
||||||
self._build_static_layout()
|
self._build_static_layout()
|
||||||
self.update_data(app_state)
|
self.update_data()
|
||||||
|
|
||||||
def _build_static_layout(self):
|
def _build_static_layout(self):
|
||||||
# ── 1. Top Section Header ──
|
# ── 1. Top Section Header ──
|
||||||
|
|
@ -280,8 +280,10 @@ class TeamView(ctk.CTkFrame):
|
||||||
self.cards_grid.grid_columnconfigure(col_idx, weight=1)
|
self.cards_grid.grid_columnconfigure(col_idx, weight=1)
|
||||||
|
|
||||||
def update_data(self, snapshot: Optional[Any] = None):
|
def update_data(self, snapshot: Optional[Any] = None):
|
||||||
from antigravity_provider.router.state_store import HubStateStore
|
from antigravity_provider.router.state_store import HubSnapshot, HubStateStore
|
||||||
if snapshot is None:
|
if not isinstance(snapshot, HubSnapshot):
|
||||||
|
# Callers have historically passed legacy app_state dicts here; a
|
||||||
|
# non-snapshot must fall back to the store, not crash the view.
|
||||||
snapshot = HubStateStore.get().get_snapshot()
|
snapshot = HubStateStore.get().get_snapshot()
|
||||||
|
|
||||||
readiness = snapshot.readiness
|
readiness = snapshot.readiness
|
||||||
|
|
|
||||||
|
|
@ -36,6 +36,10 @@ from antigravity_provider.router.codex_oauth import (
|
||||||
get_codex_oauth_session,
|
get_codex_oauth_session,
|
||||||
cancel_codex_oauth_session,
|
cancel_codex_oauth_session,
|
||||||
)
|
)
|
||||||
|
# Pulls customtkinter transitively; without this guard a headless run aborts
|
||||||
|
# collection of the entire session instead of skipping this module.
|
||||||
|
pytest.importorskip("customtkinter")
|
||||||
|
|
||||||
from antigravity_provider.router.ui.components import enable_clipboard_shortcuts, HubEntry
|
from antigravity_provider.router.ui.components import enable_clipboard_shortcuts, HubEntry
|
||||||
|
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -17,6 +17,7 @@ Broken *internal* references always fail.
|
||||||
"""
|
"""
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import ast
|
||||||
import importlib
|
import importlib
|
||||||
import pkgutil
|
import pkgutil
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
@ -81,10 +82,32 @@ def test_gui_test_modules_guard_optional_ui_dependency() -> None:
|
||||||
session ("Interrupted: 1 error during collection") instead of skipping the
|
session ("Interrupted: 1 error during collection") instead of skipping the
|
||||||
affected module, which takes the release gate down with it.
|
affected module, which takes the release gate down with it.
|
||||||
"""
|
"""
|
||||||
|
# Modules that pull the GUI toolkit in transitively when imported.
|
||||||
|
GUI_BEARING_PREFIXES = (
|
||||||
|
"customtkinter",
|
||||||
|
"antigravity_provider.router.ui",
|
||||||
|
"antigravity_provider.router.hermes_hub_app",
|
||||||
|
)
|
||||||
|
|
||||||
|
def _imports_gui(tree: ast.AST) -> bool:
|
||||||
|
for node in ast.walk(tree):
|
||||||
|
if isinstance(node, ast.Import):
|
||||||
|
if any(a.name.startswith(GUI_BEARING_PREFIXES) for a in node.names):
|
||||||
|
return True
|
||||||
|
elif isinstance(node, ast.ImportFrom):
|
||||||
|
if (node.module or "").startswith(GUI_BEARING_PREFIXES):
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
offenders: list[str] = []
|
offenders: list[str] = []
|
||||||
for path in sorted(TESTS_DIR.glob("test_*.py")):
|
for path in sorted(TESTS_DIR.glob("test_*.py")):
|
||||||
text = path.read_text(encoding="utf-8", errors="ignore")
|
text = path.read_text(encoding="utf-8", errors="ignore")
|
||||||
if "customtkinter" not in text:
|
try:
|
||||||
|
tree = ast.parse(text)
|
||||||
|
except SyntaxError:
|
||||||
|
continue
|
||||||
|
# A mention in prose is not an import; only real imports need the guard.
|
||||||
|
if not _imports_gui(tree):
|
||||||
continue
|
continue
|
||||||
if "importorskip" not in text:
|
if "importorskip" not in text:
|
||||||
offenders.append(path.name)
|
offenders.append(path.name)
|
||||||
|
|
|
||||||
71
tests/test_view_startup_contract.py
Normal file
71
tests/test_view_startup_contract.py
Normal file
|
|
@ -0,0 +1,71 @@
|
||||||
|
"""Startup contract for UI views — runs without customtkinter.
|
||||||
|
|
||||||
|
Guards the crash that took the app down on launch:
|
||||||
|
|
||||||
|
TeamView.__init__ forwarded its legacy ``app_state`` dict into
|
||||||
|
``update_data(snapshot)``. The guard there only handled ``None``, so ``{}``
|
||||||
|
slipped through and ``snapshot.readiness`` raised
|
||||||
|
``'dict' object has no attribute 'readiness'``. "Команда" is the default
|
||||||
|
view, so the failure happened before the window ever appeared.
|
||||||
|
|
||||||
|
These checks are static: they read the view sources rather than build widgets,
|
||||||
|
so they run in headless environments where the GUI toolkit is absent.
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import ast
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
VIEWS_DIR = Path(__file__).resolve().parent.parent / "src" / "antigravity_provider" / "router" / "ui" / "views"
|
||||||
|
|
||||||
|
|
||||||
|
def _view_files() -> list[Path]:
|
||||||
|
return sorted(VIEWS_DIR.glob("*_view.py"))
|
||||||
|
|
||||||
|
|
||||||
|
def _self_update_data_calls(tree: ast.AST, inside: str) -> list[ast.Call]:
|
||||||
|
"""Return self.update_data(...) calls made from the named method."""
|
||||||
|
calls: list[ast.Call] = []
|
||||||
|
for cls in (n for n in ast.walk(tree) if isinstance(n, ast.ClassDef)):
|
||||||
|
for fn in (n for n in cls.body if isinstance(n, ast.FunctionDef) and n.name == inside):
|
||||||
|
for node in ast.walk(fn):
|
||||||
|
if (
|
||||||
|
isinstance(node, ast.Call)
|
||||||
|
and isinstance(node.func, ast.Attribute)
|
||||||
|
and node.func.attr == "update_data"
|
||||||
|
and isinstance(node.func.value, ast.Name)
|
||||||
|
and node.func.value.id == "self"
|
||||||
|
):
|
||||||
|
calls.append(node)
|
||||||
|
return calls
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
@pytest.mark.parametrize("view_path", _view_files(), ids=lambda p: p.stem.replace("_view", ""))
|
||||||
|
def test_constructor_does_not_forward_app_state_to_update_data(view_path: Path) -> None:
|
||||||
|
"""A view constructor must not pass its legacy app_state into update_data."""
|
||||||
|
tree = ast.parse(view_path.read_text(encoding="utf-8"))
|
||||||
|
for call in _self_update_data_calls(tree, inside="__init__"):
|
||||||
|
assert not call.args and not call.keywords, (
|
||||||
|
f"{view_path.name}: __init__ calls self.update_data(...) with an argument. "
|
||||||
|
"update_data expects a HubSnapshot; constructors hold app_state dicts. "
|
||||||
|
"Call self.update_data() and let it pull the current snapshot."
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.unit
|
||||||
|
@pytest.mark.parametrize("view_path", _view_files(), ids=lambda p: p.stem.replace("_view", ""))
|
||||||
|
def test_update_data_guard_rejects_non_snapshot(view_path: Path) -> None:
|
||||||
|
"""update_data must fall back on anything that is not a HubSnapshot, not just None."""
|
||||||
|
source = view_path.read_text(encoding="utf-8")
|
||||||
|
if "def update_data" not in source or "snapshot" not in source:
|
||||||
|
pytest.skip("view has no snapshot-driven update_data")
|
||||||
|
if "HubStateStore.get().get_snapshot()" not in source:
|
||||||
|
pytest.skip("view does not read the snapshot store")
|
||||||
|
|
||||||
|
assert "isinstance(snapshot, HubSnapshot)" in source, (
|
||||||
|
f"{view_path.name}: update_data guards with `snapshot is None` only. "
|
||||||
|
"A legacy dict passes that check and then fails on attribute access."
|
||||||
|
)
|
||||||
Loading…
Reference in a new issue