CONTRIBUTORS.md

@@ -23,7 +23,7 @@ Below are some of the specific details of various contributions.
 * Jeffrey Phillips Freeman has acted as Lead Developer, daily contributor, and Project Owner.
 * Jeffrey Phillips Freeman has contributed the invariant merge precedence fix (#9126): restored the missing ACTION scope in ``merge_invariants()`` and ``InvariantSet.merge()``, corrected all module docstrings from ``plan > project > global`` to the spec-compliant ``plan > action > project > global``, and added comprehensive BDD test coverage for four-tier merge precedence.
 * Brent E. Edwards has contributed quality assurance, test coverage, and CI pipeline improvements.
-* HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool.
+* HAL 9000 has contributed automated implementation, bug fixes, and feature development as part of the CleverAgents automation pool, including fix for PermissionsScreen base class (#10744 / #10488): converted PermissionsScreen from a Static widget to a proper Textual Screen subclass with full keyboard bindings and action methods.
 * HAL 9000 has contributed concurrency safety improvements, including thread-safe context tier management (issue #7547) for parallel plan execution.
 * HAL 9000 has contributed the configurable agent limits refactor (#9246/#9050): replaced hardcoded ``deps[:10]`` in ``ContextAnalysisAgent`` and ``contexts[:5]`` in ``PlanGenerationGraph`` with validated constructor parameters ``max_dependencies`` (default: 10) and ``max_context_files`` (default: 5), including 12 BDD scenarios covering defaults, custom values, edge cases, and invalid-input error handling.
 * HAL 9000 has contributed the plan concurrency race-condition fix (#7989): wired `LockService` into the plan lifecycle, guarding `execute_plan()` and `apply_plan()` with plan-level advisory locks and unique per-invocation owner identities to prevent silent concurrent state corruption.

features/execution_environment.feature

@@ -129,10 +129,10 @@ Feature: Execution environment routing
 
   Scenario: CONTAINER_RESOURCE_TYPES includes expected types
     Given I import CONTAINER_RESOURCE_TYPES
-    Then it should contain "container-instance"
-    And it should contain "devcontainer-instance"
-    And it should not contain "devcontainer-file"
-    And it should not contain "git-checkout"
+    Then the container types should contain "container-instance"
+    And the container types should contain "devcontainer-instance"
+    And the container types should not contain "devcontainer-file"
+    And the container types should not contain "git-checkout"
 
   # ── ContainerUnavailableError ───────────────────────────────────────
 
@@ -145,4 +145,3 @@ Feature: Execution environment routing
     When I create a ContainerUnavailableError without project name
     Then exec-env the error message should contain "Container resource unavailable"
     And exec-env the error message should not contain "for project"
-

features/steps/execution_environment_steps.py

@@ -342,12 +342,12 @@ def step_import_types(context: Context) -> None:
     context.container_types = CONTAINER_RESOURCE_TYPES
 
 
-@then('it should contain "{value}"')
+@then('the container types should contain "{value}"')
 def step_types_contain(context: Context, value: str) -> None:
     assert value in context.container_types
 
 
-@then('it should not contain "{value}"')
+@then('the container types should not contain "{value}"')
 def step_types_not_contain(context: Context, value: str) -> None:
     assert value not in context.container_types
 

features/steps/tui_permissions_screen_steps.py

@@ -616,3 +616,69 @@ def step_left_side_empty_for_inserted(context):
 @then("the clear result should be True")
 def step_clear_result_true(context):
     assert context._clear_result is True
+
+
+# ---------------------------------------------------------------------------
+# Bug #10488: PermissionsScreen base class checks
+# ---------------------------------------------------------------------------
+
+
+@when("I check the base class of PermissionsScreen")
+def step_check_base_class(context):
+    from cleveragents.tui.permissions.screen import PermissionsScreen
+
+    context._permissions_screen_cls = PermissionsScreen
+
+
+@then("PermissionsScreen should be a subclass of textual.app.Screen")
+def step_permissions_screen_is_screen_subclass(context):
+    import importlib
+
+    from cleveragents.tui.permissions import screen as screen_module
+
+    # Check the _ScreenBase variable in the module — if textual is available,
+    # it should be textual.app.Screen; if not, the fallback is used.
+    screen_base = getattr(screen_module, "_ScreenBase", None)
+    assert screen_base is not None, (
+        "Expected screen module to have _ScreenBase variable"
+    )
+
+    try:
+        Screen = importlib.import_module("textual.app").Screen
+        # Textual is available — verify PermissionsScreen inherits from Screen
+        assert issubclass(context._permissions_screen_cls, Screen), (
+            f"Expected PermissionsScreen to be a subclass of textual.app.Screen, "
+            f"but its MRO is: {[c.__name__ for c in context._permissions_screen_cls.__mro__]}"
+        )
+        assert screen_base is Screen, (
+            f"Expected _ScreenBase to be textual.app.Screen, got {screen_base!r}"
+        )
+    except ImportError:
+        # Textual not installed — verify the module is designed to use Screen
+        # by checking that _load_screen_base is defined (not _load_static_base).
+        load_fn = getattr(screen_module, "_load_screen_base", None)
+        assert load_fn is not None, (
+            "Expected screen module to have _load_screen_base function "
+            "(not _load_static_base). The module must be designed to load "
+            "textual.app.Screen as the base class."
+        )
+
+
+@then("PermissionsScreen should have a BINDINGS class variable")
+def step_permissions_screen_has_bindings(context):
+    cls = context._permissions_screen_cls
+    assert hasattr(cls, "BINDINGS"), (
+        "Expected PermissionsScreen to have a BINDINGS class variable"
+    )
+    assert cls.BINDINGS, "Expected PermissionsScreen.BINDINGS to be non-empty"
+
+
+@then('PermissionsScreen should have action method "{method_name}"')
+def step_permissions_screen_has_action_method(context, method_name):
+    cls = context._permissions_screen_cls
+    assert hasattr(cls, method_name), (
+        f"Expected PermissionsScreen to have action method '{method_name}'"
+    )
+    assert callable(getattr(cls, method_name)), (
+        f"Expected PermissionsScreen.{method_name} to be callable"
+    )

features/tui_permissions_screen.feature

@@ -373,3 +373,27 @@ Feature: TUI PermissionsScreen
     And I record decision "allow_always" for request "req-1"
     And I clear the session decision for "local/file-write"
     Then the clear result should be True
+
+  # ── Bug #10488: PermissionsScreen must inherit from Screen ────
+
+  @tdd_issue @tdd_issue_10488
+  Scenario: Bug #10488 - PermissionsScreen inherits from textual.app.Screen
+    When I check the base class of PermissionsScreen
+    Then PermissionsScreen should be a subclass of textual.app.Screen
+
+  @tdd_issue @tdd_issue_10488
+  Scenario: Bug #10488 - PermissionsScreen has BINDINGS class variable
+    When I check the base class of PermissionsScreen
+    Then PermissionsScreen should have a BINDINGS class variable
+
+  @tdd_issue @tdd_issue_10488
+  Scenario: Bug #10488 - PermissionsScreen has action methods for keyboard bindings
+    When I check the base class of PermissionsScreen
+    Then PermissionsScreen should have action method "action_allow_once"
+    And PermissionsScreen should have action method "action_allow_always"
+    And PermissionsScreen should have action method "action_reject_once"
+    And PermissionsScreen should have action method "action_reject_always"
+    And PermissionsScreen should have action method "action_nav_next"
+    And PermissionsScreen should have action method "action_nav_prev"
+    And PermissionsScreen should have action method "action_cycle_diff"
+    And PermissionsScreen should have action method "action_dismiss_screen"

src/cleveragents/tui/permissions/screen.py

@@ -1,15 +1,17 @@
-"""PermissionsScreen widget for displaying tool permission requests with diff views.
+"""PermissionsScreen for displaying tool permission requests with diff views.
 
 Shows a split-pane layout: a file list on the left and a diff view on the right.
-Supports three diff display modes (unified, split, auto) toggled with ``d``.
+Supports three diff display modes (unified, side-by-side, context) toggled with ``d``.
 Allow/reject keyboard bindings: ``a`` allow-once, ``A`` allow-always,
 ``r`` reject-once, ``R`` reject-always.
 """
 
 from __future__ import annotations
 
+import contextlib
 import importlib
-from typing import Any
+from collections.abc import Iterator
+from typing import Any, ClassVar
 
 from cleveragents.tui.permissions.models import (
     DiffDisplayMode,
@@ -23,22 +25,22 @@ __all__ = ["PermissionsScreen"]
 # ── Optional Textual import gate ─────────────────────────────────
 
 
-def _load_static_base() -> type[Any]:
+def _load_screen_base() -> type[Any]:
     try:
-        return importlib.import_module("textual.widgets").Static
+        return importlib.import_module("textual.app").Screen
     except Exception:  # pragma: no cover
 
-        class _FallbackStatic:
+        class _FallbackScreen:
             def __init__(self, *args: object, **kwargs: object) -> None:
                 self._text = ""
 
             def update(self, text: str) -> None:
                 self._text = text
 
-        return _FallbackStatic
+        return _FallbackScreen
 
 
-_StaticBase = _load_static_base()
+_ScreenBase = _load_screen_base()
 
 # ── Diff display mode cycle ───────────────────────────────────────
 
@@ -110,11 +112,16 @@ def _render_screen(
     return f"Permission Request\n\n{file_list}\n\n{diff_panel}\n\n{status_bar}"
 
 
-# ── PermissionsScreen widget ──────────────────────────────────────
+# ── PermissionsScreen ─────────────────────────────────────────────
 
 
-class PermissionsScreen(_StaticBase):
-    """TUI widget that displays a tool permission request with a diff view.
+class PermissionsScreen(_ScreenBase):
+    """TUI screen that displays a tool permission request with a diff view.
+
+    Inherits from ``textual.app.Screen`` so it can be pushed onto the
+    Textual screen stack via ``app.push_screen()``, receive keyboard events
+    natively through ``BINDINGS``, and participate in the full Textual
+    screen lifecycle (``compose``, ``on_mount``, ``dismiss``, etc.).
 
     Layout:
     - Title: "Permission Request"
@@ -128,10 +135,21 @@ class PermissionsScreen(_StaticBase):
     - ``r``: reject once
     - ``R``: reject always (session)
     - ``j`` / ``k``: navigate file list (next / previous)
-    - ``d``: cycle diff display mode (unified → split → auto)
-    - ``escape``: dismiss (caller responsibility)
+    - ``d``: cycle diff display mode (unified → side-by-side → context)
+    - ``escape``: dismiss screen
     """
 
+    BINDINGS: ClassVar[list[tuple[str, str, str]]] = [
+        ("a", "allow_once", "Allow Once"),
+        ("A", "allow_always", "Allow Always"),
+        ("r", "reject_once", "Reject Once"),
+        ("R", "reject_always", "Reject Always"),
+        ("j", "nav_next", "Next File"),
+        ("k", "nav_prev", "Previous File"),
+        ("d", "cycle_diff", "Cycle Diff Mode"),
+        ("escape", "dismiss_screen", "Dismiss"),
+    ]
+
     def __init__(self, *args: object, **kwargs: object) -> None:
         super().__init__(*args, **kwargs)
         self._request: ToolPermissionRequest | None = None
@@ -139,7 +157,28 @@ class PermissionsScreen(_StaticBase):
         self._diff_mode: DiffDisplayMode = DiffDisplayMode.UNIFIED
         self._decision: PermissionDecision | None = None
         self._text: str = "(no permission request)"
-        self.update(self._text)
+
+    def update(self, text: str) -> None:
+        """Update the internal text representation.
+
+        Stores the rendered text for programmatic access and testing.
+        In a live Textual app, the ``compose()`` method renders the
+        content via widgets; this method keeps the ``_text`` attribute
+        in sync for non-Textual usage and testing.
+        """
+        self._text = text
+
+    def compose(self) -> Iterator[Any]:
+        """Compose the screen layout with a Static widget showing the content.
+
+        Returns a generator over Textual ``Widget`` instances (the
+        ``ComposeResult`` type), but typed as ``Iterator[Any]`` here so
+        the module stays importable in environments where ``textual`` is
+        not installed (typecheck gate, pure-domain tests).
+        """
+        with contextlib.suppress(Exception):  # pragma: no cover
+            Static = importlib.import_module("textual.widgets").Static
+            yield Static(self._text, id="permissions-content")
 
     # ── Public API ────────────────────────────────────────────────
 
@@ -238,6 +277,41 @@ class PermissionsScreen(_StaticBase):
         self._refresh()
         return decision
 
+    # ── Textual action methods ────────────────────────────────────
+
+    def action_allow_once(self) -> None:
+        """Textual action: allow once (``a`` key binding)."""
+        self.allow_once()
+
+    def action_allow_always(self) -> None:
+        """Textual action: allow always (``A`` key binding)."""
+        self.allow_always()
+
+    def action_reject_once(self) -> None:
+        """Textual action: reject once (``r`` key binding)."""
+        self.reject_once()
+
+    def action_reject_always(self) -> None:
+        """Textual action: reject always (``R`` key binding)."""
+        self.reject_always()
+
+    def action_nav_next(self) -> None:
+        """Textual action: navigate to next file (``j`` key binding)."""
+        self.navigate_next()
+
+    def action_nav_prev(self) -> None:
+        """Textual action: navigate to previous file (``k`` key binding)."""
+        self.navigate_prev()
+
+    def action_cycle_diff(self) -> None:
+        """Textual action: cycle diff display mode (``d`` key binding)."""
+        self.cycle_diff_mode()
+
+    def action_dismiss_screen(self) -> None:
+        """Textual action: dismiss the screen (``escape`` key binding)."""
+        with contextlib.suppress(Exception):  # pragma: no cover
+            self.dismiss()
+
     # ── Rendering ─────────────────────────────────────────────────
 
     def _refresh(self) -> None: