Stage 3 review: keep the main prompt inside the reservation, and gate…

… after_render The main prompt went through the same physical suspension as an external program, so every prompt gave the rows back, showed the native toolbar and reset the margins -- which is the stable-toolbar requirement inverted. The prompt the reservation exists for now keeps it; any other session is one cmd2 has not bound to the reservation and still gets the terminal to itself. Upstream fires after_render once render() returns, whatever the wrapper decided, so a skipped frame told everything downstream that a frame was on the screen. It is gated on the emission actually reaching the terminal now. Gating it exposed that the command display used that same event to mean "the display has started" -- two different questions sharing one signal. Waiting for a committed frame made starting the display depend on a cursor-position round trip, and with a terminal that never answers it never started at all. Readiness now hangs on a render attempt, which is what it was really asking about. The readiness wait is also bounded. It had no timeout, so anything that stopped the signal arriving held the command thread forever -- which is how this was found, as a hung suite rather than a failing test. A display that cannot start now says so, and the command runs without it.
tleonhardt · Sep 12, 2026 · Sep 10, 2026 · Sep 10, 2026 · Sep 10, 2026 · Sep 10, 2026
commit 471095dd5d568ef42a234509511582d53e5cc8c0
@@ -3739,7 +3739,6 @@ def _is_tty_session(session: PromptSession[str]) -> bool:
         # a DummyOutput.
         return not isinstance(session.input, DummyInput)
 
-    @command_toolbar.suspend_toolbar
     def _read_raw_input(
         self,
         prompt: Callable[[], ANSI | str] | ANSI | str,
@@ -3752,6 +3751,31 @@ def _read_raw_input(
         UI with completion and `patch_stdout` protection. Otherwise it performs
         a direct line read from `stdin`.
 
+        The command display is stopped either way, but only some prompts give the terminal
+        away with it. The main prompt is the one the reservation exists for: it renders
+        through the reserved output, and the toolbar has to still be there while the user is
+        typing -- that is what "stable across ordinary commands" means. Any other session is
+        an application prompt cmd2 has not bound to the reservation, so it gets the terminal
+        to itself, rows included.
+
+        :param prompt: the prompt text or a callable that returns the prompt.
+        :param session: the PromptSession instance to use for reading.
+        :param prompt_kwargs: additional arguments passed directly to session.prompt().
+        :return: the stripped input string.
+        :raises EOFError: if the input stream is closed or the user signals EOF (e.g., Ctrl+D)
+        """
+        owns_the_reservation = session is self.main_session
+        with self._quiesce_bottom_toolbar() if owns_the_reservation else self.suspend_bottom_toolbar():
+            return self._read_raw_input_now(prompt, session, **prompt_kwargs)
+
+    def _read_raw_input_now(
+        self,
+        prompt: Callable[[], ANSI | str] | ANSI | str,
+        session: PromptSession[str],
+        **prompt_kwargs: Any,
+    ) -> str:
+        """Read one line, with the display already stopped by the caller.
+
         :param prompt: the prompt text or a callable that returns the prompt.
         :param session: the PromptSession instance to use for reading.
         :param prompt_kwargs: additional arguments passed directly to session.prompt().

@@ -30,6 +30,10 @@
 if TYPE_CHECKING:
     from .cmd2 import Cmd
 
+#: How long to wait for the display to report that it has started. Long enough that a busy
+#: machine is not mistaken for a broken one, short enough that a command is never held forever.
+_STARTUP_TIMEOUT = 10.0
+
 _F = TypeVar("_F", bound=Callable[..., Any])
 _R = TypeVar("_R")
 
@@ -246,9 +250,29 @@ def suspend(event: KeyPressEvent) -> None:
         self._bindings = bindings
         self._suspend_binding = suspend
 
-    def _after_render(self, app: Application[str]) -> None:  # noqa: ARG002
+    def _display_started(self, app: Application[str]) -> None:  # noqa: ARG002
+        """Report that the display is up and has finished its first frame."""
+        self._ready.set()
+
+    def _display_started_without_app(self) -> None:
+        """Report readiness from a render attempt that produced no frame.
+
+        A skipped frame still means the application is running and rendering. Waiting for one
+        that commits would make starting the display depend on a cursor-position round trip,
+        and a terminal that never answers would never let the command begin.
+        """
         self._ready.set()
 
+    def _reserved_bridge(self) -> Any:
+        """Return the renderer bridge, when a reservation is holding the toolbar.
+
+        :return: the bridge, or ``None`` in legacy rendering
+        """
+        reserved = self.cmd.reserved_toolbar
+        if reserved is None or not reserved.is_active:
+            return None
+        return reserved.bridge
+
     def start(self) -> None:
         """Start rendering and protect terminal output."""
         stack = contextlib.ExitStack()
@@ -283,8 +307,15 @@ def _resume(self) -> None:
         for name, value in (("layout", self._layout), ("key_bindings", self._bindings), ("erase_when_done", True)):
             stack.callback(setattr, self.app, name, getattr(self.app, name))
             setattr(self.app, name, value)
-        self.app.after_render += self._after_render
-        stack.callback(self.app.after_render.remove_handler, self._after_render)
+        self.app.after_render += self._display_started
+        stack.callback(self.app.after_render.remove_handler, self._display_started)
+        bridge = self._reserved_bridge()
+        if bridge is not None:
+            # In reserved mode a frame can be skipped, and the after-render event is withheld
+            # for those because nothing reached the terminal. Readiness is a different
+            # question -- the display is up either way -- so it hangs on the attempt instead.
+            bridge.set_render_attempted_handler(self._display_started_without_app)
+            stack.callback(bridge.set_render_attempted_handler, None)
         context = contextvars.copy_context()
 
         def run() -> None:
@@ -301,7 +332,11 @@ def run() -> None:
 
         self._thread = threading.Thread(target=context.run, args=(run,), name="cmd2-toolbar", daemon=True)
         self._thread.start()
-        self._ready.wait()
+        if not self._ready.wait(timeout=_STARTUP_TIMEOUT):
+            # Bounded so a display that never reports itself started fails here instead of
+            # holding the command thread forever. The toolbar is cosmetic; a command waiting
+            # indefinitely on one is not a trade anyone would choose.
+            raise TimeoutError(f"the bottom toolbar did not start within {_STARTUP_TIMEOUT} seconds")
         if self._error is not None:
             raise self._error
         if self._install_serializers():

@@ -137,6 +137,12 @@ def __init__(self, renderer: "Renderer", display: "TerminalDisplay", lock: Termi
         self._bound_app: Application[Any] | None = None
         self._emission_stopped_handler: Callable[[], None] | None = None
         self._frame_committed_handler: Callable[[], object] | None = None
+        self._render_attempted_handler: Callable[[], object] | None = None
+        # Whether the last thing this bridge tried to emit actually reached the terminal.
+        self._last_emission_committed = False
+        self._after_render_event: Any = None
+        self._after_render_original: Any = None
+        self._after_render_installed: Any = None
 
     # -- what is known ---------------------------------------------------------------------
 
@@ -278,6 +284,18 @@ def set_frame_committed_handler(self, handler: "Callable[[], object]") -> None:
         """
         self._frame_committed_handler = handler
 
+    def set_render_attempted_handler(self, handler: "Callable[[], object] | None") -> None:
+        """Install what to call after each render attempt, whatever came of it.
+
+        Distinct from the committed-frame handler on purpose. "A frame reached the terminal"
+        and "the renderer has been through a frame" are different facts, and something waiting
+        for the display to start needs the second: a frame skipped while recovery is owed
+        still means the application is running and rendering.
+
+        :param handler: called after every render attempt, or ``None`` to remove it
+        """
+        self._render_attempted_handler = handler
+
     def set_emission_stopped_handler(self, handler: "Callable[[], None]") -> None:
         """Install what to call when reserved rendering has to be abandoned.
 
@@ -355,6 +373,15 @@ def bind(self, app: "Application[Any]") -> None:
         self._originals = {name: getattr(renderer, name) for name in replacements}
         self._installed = dict(replacements)
         self._bound_app = app
+        # Upstream fires this after ``render()`` returns, whatever the wrapper decided to do,
+        # so a frame the bridge skipped would still tell everything waiting on a rendered
+        # frame that one had happened -- including the command display's readiness signal.
+        self._after_render_event = app.after_render
+        self._after_render_original = app.after_render.fire
+        self._after_render_installed = self._fire_after_render_through_bridge
+        # By name, as with the renderer's methods: this replacement belongs to this event
+        # object, not to the class every application's events are built from.
+        setattr(app.after_render, "fire", self._after_render_installed)  # noqa: B010
         for name, replacement in replacements.items():
             # Set by name so the replacement lands on this instance. Assigning the class
             # attribute would change every renderer in the process, including ones cmd2 does
@@ -368,6 +395,12 @@ def unbind(self) -> None:
 
         Safe to call when nothing was bound: teardown reaches this from more than one place.
         """
+        event, self._after_render_event = self._after_render_event, None
+        if event is not None and getattr(event, "fire", None) == self._after_render_installed:
+            setattr(event, "fire", self._after_render_original)  # noqa: B010
+        self._after_render_original = None
+        self._after_render_installed = None
+
         originals, self._originals = self._originals, {}
         installed, self._installed = self._installed, {}
         for name, original in originals.items():
@@ -379,6 +412,19 @@ def unbind(self) -> None:
         self._bound_app = None
 
     def _render_through_bridge(self, app: "Application[Any]", layout: Any, is_done: bool = False) -> None:
+        """Prepare and commit one frame, telling anything waiting that an attempt was made.
+
+        :param app: the application being rendered
+        :param layout: the layout to render; upstream passes ``app.layout``
+        :param is_done: whether this is the final frame of a prompt
+        """
+        try:
+            self._render_frame(app, layout, is_done)
+        finally:
+            if self._render_attempted_handler is not None:
+                self._render_attempted_handler()
+
+    def _render_frame(self, app: "Application[Any]", layout: Any, is_done: bool = False) -> None:
         """Prepare and commit one frame in place of upstream's direct render.
 
         Runs on the UI thread, which is where recovery's callbacks belong too, so an owed
@@ -389,6 +435,7 @@ def _render_through_bridge(self, app: "Application[Any]", layout: Any, is_done:
         :param layout: the layout to render; upstream passes ``app.layout``
         :param is_done: whether this is the final frame of a prompt
         """
+        self._last_emission_committed = False
         if self._reserved_emission_stopped:
             # Abandoned, but the rows are still withheld until the owner releases them.
             # Rendering upstream directly from here would write outside the transaction and
@@ -411,9 +458,22 @@ def _render_through_bridge(self, app: "Application[Any]", layout: Any, is_done:
         if not self.commit(prepared):
             self._request_redraw()
             return
+        self._last_emission_committed = True
         if self._frame_committed_handler is not None:
             self._frame_committed_handler()
 
+    def _fire_after_render_through_bridge(self) -> None:
+        """Tell the application a frame was rendered, but only if one actually was.
+
+        A skipped frame -- recovery owed and unfinished, a preparation refused, a commit
+        retired -- emitted nothing. Everything downstream of this event believes a frame is on
+        the screen: layout metadata is published from it, and the command display treats it as
+        the signal that its first frame has been drawn.
+        """
+        if not self._last_emission_committed:
+            return
+        self._after_render_original()
+
     def _request_cursor_position_through_bridge(self) -> None:
         """Let upstream ask for the cursor, and record the request if one went out.
 
@@ -448,9 +508,11 @@ def _erase_through_bridge(self, leave_alternate_screen: bool = True) -> None:
 
         :param leave_alternate_screen: passed through to upstream
         """
+        self._last_emission_committed = False
         with self._lock.transaction("erase"):
             try:
                 self._originals["erase"](leave_alternate_screen)
+                self._last_emission_committed = True
             finally:
                 # Recorded whether or not it finished. An erase that raised part-way has still
                 # moved the cursor and cleared some of what was below it, and a stream cannot
@@ -465,9 +527,11 @@ def _clear_through_bridge(self) -> None:
         otherwise place the next frame where the prompt used to be. Cursor reports already in
         flight describe the screen before the clear and are discarded with it.
         """
+        self._last_emission_committed = False
         with self._lock.transaction("clear"):
             try:
                 self._originals["clear"]()
+                self._last_emission_committed = True
             finally:
                 self._prompt_anchor = None
                 self._invalidate_pending_cursor_reports()

@@ -806,3 +806,23 @@ def test_builtin_pager_does_not_capture_redirected_output(toolbar_app, monkeypat
         pager.assert_not_called()
     assert "Cmd2 Commands" in target.read_text(encoding="utf-8")
     assert "Cmd2 Commands" not in output.getvalue()
+
+
+def test_command_toolbar_startup_does_not_wait_forever(toolbar_app, monkeypatch, capsys) -> None:
+    """A display that never reports itself started must not hold the command thread.
+
+    The readiness signal comes from the display's own thread, so anything that stops it
+    arriving -- a render that never completes, a frame skipped forever -- would otherwise
+    block the command that is waiting to run.
+    """
+    app, _, _ = toolbar_app
+    monkeypatch.setattr(command_toolbar, "_STARTUP_TIMEOUT", 0.2)
+    monkeypatch.setattr(command_toolbar.CommandToolbar, "_display_started", lambda *args: None)
+    monkeypatch.setattr(command_toolbar.CommandToolbar, "_display_started_without_app", lambda *args: None)
+
+    ran = []
+    with app._command_toolbar_context():
+        ran.append(True)
+
+    assert ran == [True]
+    assert "did not start" in capsys.readouterr().err
@@ -1375,3 +1375,61 @@ def test_the_notification_runs_outside_the_transaction(self) -> None:
         with set_app(harness.app):
             harness.renderer.render(harness.app, harness.app.layout)
         assert seen == [None]
+
+    def test_a_skipped_frame_fires_no_after_render(self) -> None:
+        """Upstream fires the event after render() returns, whatever the wrapper decided."""
+        fired: list[int] = []
+        harness = Harness()
+        harness.bridge.bind(harness.app)
+        harness.app.after_render += lambda _app: fired.append(1)
+        harness.bridge.forget_prompt_anchor()
+        harness.bridge.require_resynchronization("a command wrote")
+
+        with set_app(harness.app):
+            harness.renderer.render(harness.app, harness.app.layout)
+            harness.app.after_render.fire()
+        assert fired == []
+
+    def test_a_committed_frame_fires_after_render(self) -> None:
+        fired: list[int] = []
+        harness = Harness()
+        harness.bridge.bind(harness.app)
+        harness.app.after_render += lambda _app: fired.append(1)
+
+        with set_app(harness.app):
+            harness.renderer.render(harness.app, harness.app.layout)
+            harness.app.after_render.fire()
+        assert fired == [1]
+
+    def test_an_erase_fires_after_render(self) -> None:
+        """It reached the terminal, so whatever waits on a frame has had one."""
+        fired: list[int] = []
+        harness = Harness()
+        harness.bridge.bind(harness.app)
+        harness.app.after_render += lambda _app: fired.append(1)
+
+        with set_app(harness.app):
+            harness.renderer.render(harness.app, harness.app.layout)
+            harness.renderer.erase()
+            harness.app.after_render.fire()
+        assert fired == [1]
+
+    def test_unbinding_restores_the_event(self) -> None:
+        fired: list[int] = []
+        harness = Harness()
+        original = harness.app.after_render.fire
+        harness.bridge.bind(harness.app)
+        harness.bridge.unbind()
+        assert harness.app.after_render.fire == original
+
+        harness.app.after_render += lambda _app: fired.append(1)
+        harness.app.after_render.fire()
+        assert fired == [1]
+
+    def test_an_event_replaced_while_bound_is_left_alone(self) -> None:
+        harness = Harness()
+        harness.bridge.bind(harness.app)
+        replacement = lambda: None  # noqa: E731
+        harness.app.after_render.fire = replacement  # type: ignore[method-assign]
+        harness.bridge.unbind()
+        assert harness.app.after_render.fire is replacement