Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/conductor/cli/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ async def _run_replay() -> None:
await dashboard.start()
if is_verbose():
console.print(f"\n[bold green]▶ Replay dashboard:[/] {dashboard.url}\n")
console.print("[dim]Press Ctrl+C to exit[/dim]\n")
console.print("[dim]Press Ctrl+C to exit[/dim]\n")

try:
await asyncio.Event().wait()
Expand All @@ -1005,7 +1005,8 @@ async def _run_replay() -> None:
try:
asyncio.run(_run_replay())
except KeyboardInterrupt:
console.print("\n[dim]Replay stopped.[/dim]")
if is_verbose():
console.print("\n[dim]Replay stopped.[/dim]")


@app.command()
Expand Down
41 changes: 39 additions & 2 deletions src/conductor/cli/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,45 @@
if TYPE_CHECKING:
from conductor.events import WorkflowEvent

# Verbose console for logging (stderr)
_verbose_console = Console(stderr=True, highlight=False)

# Verbose console for logging (stderr).
#
# This subclass enforces the global --silent contract ("No progress output.
# Only JSON result on stdout.") at the source: every ``.print()`` call on a
# ``_SilentAwareConsole`` becomes a no-op when ``is_verbose()`` is False, so
# individual call sites do not each have to remember to gate themselves
# (see issue #209, follow-up to #203/#211). File logging is unaffected —
# ``_file_console`` is a separate Console wired up by ``init_file_logging``.
#
# Scope: only ``.print()`` is gated. Other Rich ``Console`` output methods
# (``.log()``, ``.rule()``, ``.status()``, ``.print_json()``, etc.) would
# silently bypass ``--silent`` if used on this instance. All current call
# sites in this module use only ``.print``; if you introduce a new one,
# either route it through ``.print`` or extend this subclass.
class _SilentAwareConsole(Console):
"""``Console`` that honors ``--silent`` at the print level.

The instance is locked to ``stderr=True`` to preserve the contract that
``--silent`` runs emit JSON on stdout with nothing else; routing gated
output to stdout would corrupt that channel.
"""

def __init__(self, **kwargs: Any) -> None:
# Lock stderr=True; everything else (highlight, width, etc.) is
# caller-tunable.
kwargs.pop("stderr", None)
super().__init__(stderr=True, **kwargs)

def print(self, *args: Any, **kwargs: Any) -> None:
# Lazy import to avoid the cli.run -> cli.app import cycle at module
# load time.
from conductor.cli.app import is_verbose

if is_verbose():
super().print(*args, **kwargs)


_verbose_console = _SilentAwareConsole(highlight=False)

# File console for file logging (None when not active)
_file_console: Console | None = None
Expand Down
58 changes: 58 additions & 0 deletions tests/test_cli/test_logging.py
Original file line number Diff line number Diff line change
Expand Up @@ -1749,3 +1749,61 @@ def test_agent_timeout_event_triggers_log(self) -> None:
assert "timed out" in output_text
finally:
verbose_mode.reset(token)


class TestSilentAwareConsole:
"""Tests for the ``_SilentAwareConsole`` subclass (issue #209).

The verbose console used by ``run`` / ``resume`` is silent-aware at the
source: every ``.print(...)`` call no-ops when ``is_verbose()`` is False.
This guarantees ``--silent`` compliance for *every* call site in
``conductor.cli.run`` without each site having to remember to gate
itself.
"""

def test_print_writes_when_verbose(self) -> None:
"""Verbose mode (the default) lets ``print`` reach the underlying stream."""
from io import StringIO

from conductor.cli.run import _SilentAwareConsole

sink = StringIO()
console = _SilentAwareConsole(file=sink, force_terminal=True, no_color=True)

token = verbose_mode.set(True)
try:
console.print("hello world")
finally:
verbose_mode.reset(token)

assert "hello world" in sink.getvalue()

def test_print_suppresses_when_silent(self) -> None:
"""Silent mode (``verbose_mode=False``) makes ``print`` a no-op."""
from io import StringIO

from conductor.cli.run import _SilentAwareConsole

sink = StringIO()
console = _SilentAwareConsole(file=sink, force_terminal=True, no_color=True)

token = verbose_mode.set(False)
try:
console.print("hello world")
console.print("[bold yellow]Warning:[/bold yellow] something failed")
console.print() # empty-arg call must also be a no-op
finally:
verbose_mode.reset(token)

assert sink.getvalue() == ""

def test_module_verbose_console_is_silent_aware(self) -> None:
"""Module-level ``_verbose_console`` is an instance of the silent-aware subclass.

Guards against a future refactor accidentally swapping it back to a
plain ``rich.console.Console``, which is the regression issue #209
is about.
"""
from conductor.cli.run import _SilentAwareConsole, _verbose_console

assert isinstance(_verbose_console, _SilentAwareConsole)
143 changes: 142 additions & 1 deletion tests/test_cli/test_replay_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
import json
import re
from pathlib import Path
from unittest.mock import AsyncMock, patch
from typing import Any
from unittest.mock import AsyncMock, MagicMock, patch

from typer.testing import CliRunner

Expand Down Expand Up @@ -81,3 +82,143 @@ def test_custom_port_option(self) -> None:
result = runner.invoke(app, ["replay", "--help"])
clean = _ANSI_RE.sub("", result.output)
assert "--web-port" in clean


class TestReplaySilentCompliance:
"""Regression tests for issue #209.

PR #211 gated the dashboard URL print in ``_run_replay`` behind
``is_verbose()`` but missed the ``Press Ctrl+C to exit`` and
``Replay stopped`` prints. Under ``--silent`` those messages still
leaked to stderr, violating the ``--silent`` contract ("No progress
output. Only JSON result on stdout.").

Two pairs of tests are needed because there are two stderr-leak
paths in the ``replay`` command:

* **Inner path** — prints inside ``_run_replay`` after the dashboard
starts (``app.py:994-996``). Exercised by faking
``asyncio.Event().wait()`` to raise ``CancelledError``.
* **Outer path** — the ``Replay stopped`` print inside the
``except KeyboardInterrupt`` handler that wraps ``asyncio.run``
(``app.py:1007-1009``). Exercised by making ``asyncio.run`` itself
raise ``KeyboardInterrupt``.

Each path also has a verbose counterpart to guard against an
over-eager gate suppressing the message when the user did NOT pass
``--silent``.
"""

@staticmethod
def _mock_replay_dashboard() -> MagicMock:
"""Build a dashboard mock with async ``start``/``stop`` no-ops."""
mock_dashboard = MagicMock()
mock_dashboard.url = "http://127.0.0.1:9999"
mock_dashboard.start = AsyncMock()
mock_dashboard.stop = AsyncMock()
return mock_dashboard

@staticmethod
def _patch_inner_wait_to_cancel() -> Any:
"""Patch ``asyncio.Event`` so ``await ... .wait()`` raises ``CancelledError``.

Without this the replay command hangs forever on the
``await asyncio.Event().wait()`` it uses to keep the dashboard
alive after start.
"""
import asyncio

mock_event = MagicMock()
mock_event.wait = AsyncMock(side_effect=asyncio.CancelledError())
return patch("asyncio.Event", return_value=mock_event)

# ----- inner path: prints inside _run_replay -----

def test_silent_replay_suppresses_dashboard_messages(self, tmp_path: Path) -> None:
"""``--silent replay`` must not leak the dashboard URL or hint to stderr."""
log_path = _write_log_file(tmp_path)
mock_dashboard = self._mock_replay_dashboard()

with (
patch(
"conductor.web.replay.ReplayDashboard", return_value=mock_dashboard
) as mock_dashboard_cls,
self._patch_inner_wait_to_cancel(),
):
result = runner.invoke(app, ["--silent", "replay", str(log_path)])

assert result.exit_code == 0, result.output
assert mock_dashboard_cls.called
stderr = _ANSI_RE.sub("", result.stderr)
assert "Replay dashboard" not in stderr
assert "Press Ctrl+C" not in stderr
assert "http://127.0.0.1:9999" not in stderr

def test_verbose_replay_prints_dashboard_messages(self, tmp_path: Path) -> None:
"""Sanity counterpart: without ``--silent`` the messages still appear."""
log_path = _write_log_file(tmp_path)
mock_dashboard = self._mock_replay_dashboard()

with (
patch("conductor.web.replay.ReplayDashboard", return_value=mock_dashboard),
self._patch_inner_wait_to_cancel(),
):
result = runner.invoke(app, ["replay", str(log_path)])

assert result.exit_code == 0, result.output
stderr = _ANSI_RE.sub("", result.stderr)
assert "Replay dashboard" in stderr
assert "http://127.0.0.1:9999" in stderr
assert "Press Ctrl+C" in stderr

def test_quiet_replay_prints_dashboard_messages(self, tmp_path: Path) -> None:
"""``--quiet`` (MINIMAL) must NOT suppress.

Locks in the contract from ``app.py:204``
(``verbose_mode.set(verbosity != ConsoleVerbosity.SILENT)``) — if
a future refactor changes that line to ``== ConsoleVerbosity.FULL``,
MINIMAL mode would start swallowing every progress message, which
is not what ``--quiet`` is supposed to mean.
"""
log_path = _write_log_file(tmp_path)
mock_dashboard = self._mock_replay_dashboard()

with (
patch("conductor.web.replay.ReplayDashboard", return_value=mock_dashboard),
self._patch_inner_wait_to_cancel(),
):
result = runner.invoke(app, ["--quiet", "replay", str(log_path)])

assert result.exit_code == 0, result.output
stderr = _ANSI_RE.sub("", result.stderr)
assert "Replay dashboard" in stderr
assert "http://127.0.0.1:9999" in stderr

# ----- outer path: print inside the KeyboardInterrupt handler -----

def test_silent_replay_suppresses_keyboardinterrupt_message(self, tmp_path: Path) -> None:
"""``--silent replay`` must not leak ``Replay stopped`` on Ctrl+C.

Drives the outer ``except KeyboardInterrupt`` branch by making
``asyncio.run`` itself raise — this is robust against future
refactors of the wait primitive inside ``_run_replay``.
"""
log_path = _write_log_file(tmp_path)

with patch("asyncio.run", side_effect=KeyboardInterrupt()):
result = runner.invoke(app, ["--silent", "replay", str(log_path)])

assert result.exit_code == 0, result.output
stderr = _ANSI_RE.sub("", result.stderr)
assert "Replay stopped" not in stderr

def test_verbose_replay_prints_keyboardinterrupt_message(self, tmp_path: Path) -> None:
"""Sanity counterpart: without ``--silent`` Ctrl+C still prints the hint."""
log_path = _write_log_file(tmp_path)

with patch("asyncio.run", side_effect=KeyboardInterrupt()):
result = runner.invoke(app, ["replay", str(log_path)])

assert result.exit_code == 0, result.output
stderr = _ANSI_RE.sub("", result.stderr)
assert "Replay stopped" in stderr
Loading