mirror of
https://github.com/usestrix/strix.git
synced 2026-10-28 06:23:07 +00:00
fix(tui): guard app-level UI work with is_running
StrixTUIApp subclasses textual.app.App, whose is_mounted is a method taking a widget, not the Widget.is_mounted property. A bare self.is_mounted is a bound method object and always truthy, so every if-not-self.is_mounted guard was dead code. Switch the 13 app-level lifecycle checks to App.is_running. Add regression coverage that verifies the Textual API distinction, locks all 13 replacements, and proves a stopped StrixTUIApp action returns before querying the widget tree.
This commit is contained in:
parent
27f9750cdc
commit
38de25fefc
2 changed files with 91 additions and 13 deletions
|
|
@ -867,7 +867,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
yield SplashScreen(id="splash_screen")
|
||||
|
||||
def watch_show_splash(self, show_splash: bool) -> None:
|
||||
if not show_splash and self.is_mounted:
|
||||
if not show_splash and self.is_running:
|
||||
try:
|
||||
splash = self.query_one("#splash_screen")
|
||||
splash.remove()
|
||||
|
|
@ -939,7 +939,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
if len(self.screen_stack) > 1 or self.show_splash:
|
||||
return
|
||||
|
||||
if not self.is_mounted:
|
||||
if not self.is_running:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
@ -954,7 +954,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
if len(self.screen_stack) > 1 or self.show_splash:
|
||||
return
|
||||
|
||||
if not self.is_mounted:
|
||||
if not self.is_running:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
@ -986,7 +986,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
if len(self.screen_stack) > 1:
|
||||
return
|
||||
|
||||
if not self.is_mounted:
|
||||
if not self.is_running:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
@ -1110,7 +1110,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
return self._get_rendered_events_content(events), "chat-content"
|
||||
|
||||
def _update_chat_view(self) -> None:
|
||||
if len(self.screen_stack) > 1 or self.show_splash or not self.is_mounted:
|
||||
if len(self.screen_stack) > 1 or self.show_splash or not self.is_running:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
@ -1467,7 +1467,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
if len(self.screen_stack) > 1 or self.show_splash:
|
||||
return
|
||||
|
||||
if not self.is_mounted:
|
||||
if not self.is_running:
|
||||
return
|
||||
|
||||
self._displayed_events.clear()
|
||||
|
|
@ -1542,7 +1542,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
if len(self.screen_stack) > 1 or self.show_splash:
|
||||
return
|
||||
|
||||
if not self.is_mounted:
|
||||
if not self.is_running:
|
||||
return
|
||||
|
||||
agent_id = agent_data["id"]
|
||||
|
|
@ -1679,7 +1679,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
if len(self.screen_stack) > 1 or self.show_splash:
|
||||
return
|
||||
|
||||
if not self.is_mounted:
|
||||
if not self.is_running:
|
||||
return
|
||||
|
||||
node = event.node
|
||||
|
|
@ -1699,7 +1699,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
if len(self.screen_stack) > 1 or self.show_splash:
|
||||
return
|
||||
|
||||
if not self.is_mounted:
|
||||
if not self.is_running:
|
||||
return
|
||||
|
||||
node = event.node
|
||||
|
|
@ -1748,7 +1748,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
return "Unknown Agent"
|
||||
|
||||
def action_toggle_help(self) -> None:
|
||||
if self.show_splash or not self.is_mounted:
|
||||
if self.show_splash or not self.is_running:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
@ -1766,7 +1766,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
self.push_screen(HelpScreen())
|
||||
|
||||
async def action_request_quit(self) -> None:
|
||||
if self.show_splash or not self.is_mounted:
|
||||
if self.show_splash or not self.is_running:
|
||||
await self.action_custom_quit()
|
||||
return
|
||||
|
||||
|
|
@ -1782,7 +1782,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
self.push_screen(QuitScreen())
|
||||
|
||||
def action_stop_selected_agent(self) -> None:
|
||||
if self.show_splash or not self.is_mounted:
|
||||
if self.show_splash or not self.is_running:
|
||||
return
|
||||
|
||||
if len(self.screen_stack) > 1:
|
||||
|
|
@ -1941,7 +1941,7 @@ class StrixTUIApp(App): # type: ignore[misc]
|
|||
return True
|
||||
|
||||
def on_resize(self, event: events.Resize) -> None:
|
||||
if self.show_splash or not self.is_mounted:
|
||||
if self.show_splash or not self.is_running:
|
||||
return
|
||||
|
||||
try:
|
||||
|
|
|
|||
78
tests/test_tui_mount_guard.py
Normal file
78
tests/test_tui_mount_guard.py
Normal file
|
|
@ -0,0 +1,78 @@
|
|||
"""Regression tests for the TUI's app-lifecycle guards.
|
||||
|
||||
``StrixTUIApp`` subclasses ``textual.app.App``, whose ``is_mounted`` is a
|
||||
*method* taking a widget — not the ``Widget.is_mounted`` property. A bare
|
||||
``self.is_mounted`` on the app is therefore a bound method object, which is
|
||||
always truthy, so ``if not self.is_mounted: return`` never returns. The
|
||||
app-level guards need ``App.is_running`` to detect a stopped app.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import inspect
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
from textual.app import App
|
||||
from textual.widget import Widget
|
||||
|
||||
from strix.interface.tui import app as tui_app
|
||||
|
||||
|
||||
# App-level guards converted from the always-truthy self.is_mounted.
|
||||
_CONVERTED_GUARDS = 13
|
||||
|
||||
|
||||
class StoppedTuiState:
|
||||
show_splash = False
|
||||
is_running = False
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.query_count = 0
|
||||
|
||||
def is_mounted(self, _widget: object) -> bool:
|
||||
return False
|
||||
|
||||
def query_one(self, *_args: object, **_kwargs: object) -> None:
|
||||
self.query_count += 1
|
||||
raise ValueError("the TUI is not mounted")
|
||||
|
||||
|
||||
def test_app_is_mounted_is_a_method_that_needs_a_widget() -> None:
|
||||
"""The API shape that caused the bug: truthy when read without calling it."""
|
||||
assert inspect.isfunction(inspect.getattr_static(App, "is_mounted"))
|
||||
assert "widget" in inspect.signature(App.is_mounted).parameters
|
||||
assert bool(App().is_mounted) is True
|
||||
|
||||
|
||||
def test_widget_is_mounted_is_a_property() -> None:
|
||||
"""``Widget.is_mounted`` *is* a property — the source of the confusion."""
|
||||
assert isinstance(inspect.getattr_static(Widget, "is_mounted"), property)
|
||||
|
||||
|
||||
def test_app_is_running_is_a_property() -> None:
|
||||
assert isinstance(inspect.getattr_static(App, "is_running"), property)
|
||||
|
||||
|
||||
def test_tui_app_never_reads_is_mounted_off_itself() -> None:
|
||||
"""``self.is_mounted`` in the TUI is always a bug; widget access is fine."""
|
||||
source = Path(str(tui_app.__file__)).read_text(encoding="utf-8")
|
||||
offenders = [
|
||||
f"{number}: {line.strip()}"
|
||||
for number, line in enumerate(source.splitlines(), start=1)
|
||||
if "self.is_mounted" in line
|
||||
]
|
||||
|
||||
assert not offenders, "self.is_mounted is always truthy; use self.is_running:\n" + "\n".join(
|
||||
offenders
|
||||
)
|
||||
# A floor, not an exact count: new guards are welcome, silent removal is not.
|
||||
assert source.count("self.is_running") >= _CONVERTED_GUARDS
|
||||
|
||||
|
||||
def test_real_tui_action_returns_before_query_when_app_is_stopped() -> None:
|
||||
stopped_tui_state = StoppedTuiState()
|
||||
|
||||
tui_app.StrixTUIApp.action_toggle_help(cast("tui_app.StrixTUIApp", stopped_tui_state))
|
||||
|
||||
assert stopped_tui_state.query_count == 0
|
||||
Loading…
Add table
Reference in a new issue