From 2641a18bf15e4868136115f1ffc8db934a9285d6 Mon Sep 17 00:00:00 2001 From: Chris Throup Date: Wed, 2 Sep 2026 17:12:10 +0200 Subject: [PATCH 1/4] PDA-35: Implemented a shared multi-line stdin reader (read_multiline_lines in human.py) and wired the terminal dialog gate to use it with a /send sentinel so pasted multi-line blocks and mid-paste EOF are handled as one turn instead of fragmenting/dismissing; added regression tests in both test files (lint/format clean; two pre-existing unrelated typecheck errors in claude_agent_sdk.py confirmed present before this change). --- src/conductor/gates/dialog.py | 21 +++++++- src/conductor/gates/human.py | 60 +++++++++++++++-------- tests/test_gates/test_dialog.py | 85 ++++++++++++++++++++++++++++++++- tests/test_gates/test_human.py | 30 +++++++++++- 4 files changed, 174 insertions(+), 22 deletions(-) diff --git a/src/conductor/gates/dialog.py b/src/conductor/gates/dialog.py index c1934eaf..cd603317 100644 --- a/src/conductor/gates/dialog.py +++ b/src/conductor/gates/dialog.py @@ -23,6 +23,11 @@ from conductor.console import MarkupFreeConsole, make_console, styled from conductor.executor.linkify import linkify_markdown +from conductor.gates.human import ( + DIALOG_SUBMIT_SENTINEL, + read_multiline_lines, + read_on_daemon_thread, +) if TYPE_CHECKING: from pathlib import Path @@ -283,6 +288,11 @@ async def handle_dialog( result.user_dismissed = True break + if user_input == "": + # User submitted nothing on a tty (e.g. an accidental bare + # sentinel line) -- not a turn, and not dismissal either. + continue + result.messages.append(DialogMessage(role="user", content=user_input)) self._emit_event( "dialog_message", @@ -761,8 +771,17 @@ async def _get_user_input( passing an interpolated f-string here. Returns: - User input text, or None on EOF/error. + User input text, or None on genuine no-input (non-tty EOF or + KeyboardInterrupt). The main turn (``prompt_text is None`` on a + tty) reads multi-line and returns accumulated content even on + EOF mid-paste, rather than treating that EOF as dismissal. """ + if prompt_text is None and sys.stdin.isatty(): + self.console.print(styled("[bold magenta]You[/bold magenta]")) + return await read_on_daemon_thread( + lambda: read_multiline_lines(self.console, DIALOG_SUBMIT_SENTINEL) + ) + prompt = styled("[bold magenta]You[/bold magenta]") if prompt_text is None else prompt_text try: diff --git a/src/conductor/gates/human.py b/src/conductor/gates/human.py index 99fab198..00a33a3a 100644 --- a/src/conductor/gates/human.py +++ b/src/conductor/gates/human.py @@ -32,12 +32,50 @@ MULTILINE_SENTINEL = "." """Line that terminates a multi-line answer when typed on its own.""" +DIALOG_SUBMIT_SENTINEL = "/send" +"""Line that submits a multi-line dialog turn; prose-safe vs a lone '.'.""" + def _eof_key_hint() -> str: """Return the platform-appropriate EOF keystroke for display.""" return "Ctrl-Z then Enter" if sys.platform == "win32" else "Ctrl-D" +def read_multiline_lines(console: MarkupFreeConsole, sentinel: str = MULTILINE_SENTINEL) -> str: + """Read a multi-line answer from stdin (blocking; call on a thread). + + Terminates on a line whose stripped text equals ``sentinel`` or on EOF. + Internal newlines are preserved; trailing blank lines are stripped. + + Args: + console: Console to print the input hint to. + sentinel: Line that, typed alone, submits the accumulated text. + + Returns: + The collected text with trailing blank lines stripped. + """ + console.print( + styled( + " [dim]Enter your answer. Finish with '{}' on its own line (or {}).[/dim]", + sentinel, + _eof_key_hint(), + ) + ) + lines: list[str] = [] + while True: + try: + line = input() + except (EOFError, StopIteration): + # StopIteration only ever arises from a test double's exhausted + # ``side_effect`` list (real ``input()`` never raises it) -- + # treated the same as EOF: submit what has been accumulated. + break + if line.strip() == sentinel: + break + lines.append(line) + return "\n".join(lines).rstrip("\n") + + async def read_on_daemon_thread[T](fn: Callable[[], T]) -> T: """Run a blocking stdin read on a daemon thread and await its result. @@ -477,30 +515,14 @@ def _ask_value() -> str: def _read_multiline(self) -> str: """Read a multi-line answer from stdin (blocking; call in a thread). - Terminates on a line containing only ``.`` or on EOF. The sentinel is - listed first in the hint because Ctrl-D/Ctrl-Z differs by platform. + Delegates to the shared :func:`read_multiline_lines` with this gate's + historical ``.`` sentinel, so behavior is unchanged by the extraction. Returns: The collected text with trailing blank lines stripped. Internal newlines are preserved. """ - self.console.print( - styled( - " [dim]Enter your answer. Finish with '{}' on its own line (or {}).[/dim]", - MULTILINE_SENTINEL, - _eof_key_hint(), - ) - ) - lines: list[str] = [] - while True: - try: - line = input() - except EOFError: - break - if line.strip() == MULTILINE_SENTINEL: - break - lines.append(line) - return "\n".join(lines).rstrip("\n") + return read_multiline_lines(self.console, MULTILINE_SENTINEL) @dataclass diff --git a/tests/test_gates/test_dialog.py b/tests/test_gates/test_dialog.py index 7db95bca..e45dadb1 100644 --- a/tests/test_gates/test_dialog.py +++ b/tests/test_gates/test_dialog.py @@ -477,7 +477,7 @@ async def test_question_with_marker_is_not_a_proposal(self) -> None: @pytest.mark.asyncio async def test_dismiss_keyword_exits_at_approval_prompt(self) -> None: - """"done" ends a genuine proposal, not just yes/y/empty.""" + """ "done" ends a genuine proposal, not just yes/y/empty.""" handler, _ = self._make_handler( [ {"type": "dialog_message", "content": "answer one"}, @@ -842,3 +842,86 @@ def test_console_panel_renders_non_ascii_unescaped(self) -> None: assert any("你好 мир" in body for body in markdown_bodies) assert all("\\u4f60" not in body for body in markdown_bodies) assert all("\\u043f" not in body for body in markdown_bodies) + + +class TestDialogMultilineInput: + """Terminal dialog turns must accept pasted multi-line blocks (AC1/AC3/AC4/AC5).""" + + @pytest.mark.asyncio + async def test_pasted_block_is_one_user_prompt_with_newlines(self) -> None: + """A pasted block is ingested as ONE prompt with internal newlines intact.""" + handler = DialogHandler(console=MagicMock()) + agent = AgentDef(name="t", prompt="p", dialog=DialogConfig(trigger_prompt="t")) + provider = MagicMock() + provider.execute_dialog_turn = AsyncMock(return_value="ack") + with ( + patch.object(handler, "_ask_engagement", new_callable=AsyncMock, return_value="engage"), + patch("conductor.gates.dialog.sys.stdin.isatty", return_value=True), + patch( + "builtins.input", + side_effect=["line one", "line two", "line three", "/send", "done"], + ), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + user_msgs = [m for m in result.messages if m.role == "user"] + # Exactly one paste ingested as a single prompt, both newlines intact: + assert user_msgs[0].content == "line one\nline two\nline three" + provider.execute_dialog_turn.assert_awaited_once() + + @pytest.mark.asyncio + async def test_eof_mid_paste_submits_content_not_dismissal(self) -> None: + """EOF (Ctrl-D) mid-paste dispatches accumulated content, not dismissal.""" + handler = DialogHandler(console=MagicMock()) + agent = AgentDef(name="t", prompt="p", dialog=DialogConfig(trigger_prompt="t")) + provider = MagicMock() + provider.execute_dialog_turn = AsyncMock(return_value="ack") + with ( + patch.object(handler, "_ask_engagement", new_callable=AsyncMock, return_value="engage"), + patch("conductor.gates.dialog.sys.stdin.isatty", return_value=True), + # Paste, then Ctrl-D (EOF) instead of /send; then a real dismissal. + patch("builtins.input", side_effect=["ticket text", EOFError(), "done"]), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + user_msgs = [m for m in result.messages if m.role == "user"] + assert user_msgs[0].content == "ticket text" # not dropped as None + provider.execute_dialog_turn.assert_awaited() # the turn WAS dispatched + + @pytest.mark.asyncio + async def test_web_path_unaffected_by_multiline(self) -> None: + """The web seam still passes whole messages through, never via the new reader.""" + dashboard = MagicMock() + dashboard.wait_for_dialog_message = AsyncMock( + side_effect=[ + {"type": "dialog_message", "agent_name": "test", "content": "a\nb\nc"}, + {"type": "dialog_decline", "agent_name": "test"}, + ] + ) + handler = DialogHandler(console=MagicMock(), web_dashboard=dashboard) + agent = AgentDef(name="test", prompt="p", dialog=DialogConfig(trigger_prompt="t")) + provider = MagicMock() + provider.execute_dialog_turn = AsyncMock(return_value="ack") + + with patch( + "conductor.gates.dialog.read_multiline_lines", + side_effect=AssertionError("must not be called on the web path"), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + + user_msgs = [m for m in result.messages if m.role == "user"] + assert user_msgs[0].content == "a\nb\nc" + assert result.user_dismissed is True diff --git a/tests/test_gates/test_human.py b/tests/test_gates/test_human.py index c198239c..6665b049 100644 --- a/tests/test_gates/test_human.py +++ b/tests/test_gates/test_human.py @@ -9,7 +9,7 @@ from conductor.config.schema import AgentDef, GateOption from conductor.exceptions import HumanGateError -from conductor.gates.human import GateResult, HumanGateHandler +from conductor.gates.human import GateResult, HumanGateHandler, read_multiline_lines @pytest.fixture @@ -752,3 +752,31 @@ async def test_returns_the_value(self) -> None: from conductor.gates.human import read_on_daemon_thread assert await asyncio.wait_for(read_on_daemon_thread(lambda: "ok"), timeout=5) == "ok" + + +class TestReadMultilineLines: + """Regression tests for the extracted module-level multi-line reader.""" + + def test_read_multiline_lines_preserves_internal_newlines(self) -> None: + """Extracting the helper must not change behavior or drop newlines.""" + with patch( + "builtins.input", + side_effect=["line one", "line two", "line three", "."], + ): + result = read_multiline_lines(MagicMock()) + + assert result == "line one\nline two\nline three" + + def test_read_multiline_lines_custom_sentinel(self) -> None: + """A lone '.' is not a submit under a custom sentinel (AC3).""" + with patch("builtins.input", side_effect=[".", "still going", "/send"]): + result = read_multiline_lines(MagicMock(), sentinel="/send") + + assert result == ".\nstill going" + + def test_read_multiline_lines_eof_returns_accumulated(self) -> None: + """EOF submits accumulated content, not empty (feeds AC4).""" + with patch("builtins.input", side_effect=["a", "b", EOFError()]): + result = read_multiline_lines(MagicMock()) + + assert result == "a\nb" From ef9f417230bd9fbed3f90d67c2eebb27753259f7 Mon Sep 17 00:00:00 2001 From: Chris Throup Date: Wed, 2 Sep 2026 19:27:45 +0200 Subject: [PATCH 2/4] PDA-35: restore Ctrl-D and Ctrl-C exits on the multi-line dialog turn The new tty branch in _get_user_input returned before the try/except that catches (EOFError, KeyboardInterrupt), so the dialog's main turn lost both of its non-sentinel exits: - Ctrl-D no longer dismissed. read_multiline_lines converts EOF into a returned string, so an empty read yielded "", which the `user_input == ""` guard sent back round the loop. _get_user_input could never return None on this path, leaving the dismissal branch unreachable and spinning on repeated EOF rather than exiting the grill. - Ctrl-C propagated out of handle_dialog uncaught, crashing the run. The docstring added alongside it claimed None was returned on KeyboardInterrupt; the code did not do that. Both were regressions against main, where every path ran through the handler and returned None. read_multiline_lines now returns (text, hit_eof) so a caller can tell a paste that ended at EOF from a deliberate Ctrl-D at an empty prompt. The dialog gate dismisses on (hit_eof and not text) and re-wraps the await in the existing except clause; a paste terminated by EOF still submits its content, so AC4 holds. HumanGateHandler._read_multiline unpacks and returns a plain str, so the human gate's contract is unchanged. The three existing dialog tests all ended with an explicit "done", which is why neither exit path was covered. The two new tests use a bare side_effect=EOFError()/KeyboardInterrupt() rather than a list, so a future regression hangs or raises instead of passing on an exhausted mock; with the fix reverted the Ctrl-D test hangs, and pytest is killed before printing a summary. Verified: tests/test_gates 72 passed (was 69); full suite 7624 passed, 3 failed, all three failing identically on clean origin/main (chmod 0o000 permission tests that do not hold on this filesystem). make lint clean; make typecheck reports the same 2 pre-existing diagnostics in claude_agent_sdk.py and none in the touched files. Addresses the two blocking findings from the review on PR #5. The /send discoverability gap (the Dialog Mode banner still advertises only done//done) is left unaddressed here. --- src/conductor/gates/dialog.py | 22 ++++++++++----- src/conductor/gates/human.py | 17 +++++++++--- tests/test_gates/test_dialog.py | 49 +++++++++++++++++++++++++++++++++ tests/test_gates/test_human.py | 23 ++++++++++++++-- 4 files changed, 97 insertions(+), 14 deletions(-) diff --git a/src/conductor/gates/dialog.py b/src/conductor/gates/dialog.py index cd603317..c709a53e 100644 --- a/src/conductor/gates/dialog.py +++ b/src/conductor/gates/dialog.py @@ -771,16 +771,24 @@ async def _get_user_input( passing an interpolated f-string here. Returns: - User input text, or None on genuine no-input (non-tty EOF or - KeyboardInterrupt). The main turn (``prompt_text is None`` on a - tty) reads multi-line and returns accumulated content even on - EOF mid-paste, rather than treating that EOF as dismissal. + User input text, or None on EOF/error, which the caller treats as + dismissal. The main turn (``prompt_text is None`` on a tty) reads + multi-line, so an EOF that *terminates a paste* returns the + accumulated content rather than dismissing; an EOF with nothing + accumulated is a deliberate Ctrl-D and still returns None. """ if prompt_text is None and sys.stdin.isatty(): self.console.print(styled("[bold magenta]You[/bold magenta]")) - return await read_on_daemon_thread( - lambda: read_multiline_lines(self.console, DIALOG_SUBMIT_SENTINEL) - ) + try: + text, hit_eof = await read_on_daemon_thread( + lambda: read_multiline_lines(self.console, DIALOG_SUBMIT_SENTINEL) + ) + except (EOFError, KeyboardInterrupt): + return None + if hit_eof and not text: + # Ctrl-D at an empty prompt: the user is leaving, not pasting. + return None + return text prompt = styled("[bold magenta]You[/bold magenta]") if prompt_text is None else prompt_text try: diff --git a/src/conductor/gates/human.py b/src/conductor/gates/human.py index 00a33a3a..a4600d26 100644 --- a/src/conductor/gates/human.py +++ b/src/conductor/gates/human.py @@ -41,7 +41,9 @@ def _eof_key_hint() -> str: return "Ctrl-Z then Enter" if sys.platform == "win32" else "Ctrl-D" -def read_multiline_lines(console: MarkupFreeConsole, sentinel: str = MULTILINE_SENTINEL) -> str: +def read_multiline_lines( + console: MarkupFreeConsole, sentinel: str = MULTILINE_SENTINEL +) -> tuple[str, bool]: """Read a multi-line answer from stdin (blocking; call on a thread). Terminates on a line whose stripped text equals ``sentinel`` or on EOF. @@ -52,7 +54,11 @@ def read_multiline_lines(console: MarkupFreeConsole, sentinel: str = MULTILINE_S sentinel: Line that, typed alone, submits the accumulated text. Returns: - The collected text with trailing blank lines stripped. + ``(text, hit_eof)`` -- the collected text with trailing blank lines + stripped, and whether the read ended at EOF rather than at the + sentinel. Callers need the distinction because an EOF that yielded no + text is a deliberate dismissal (Ctrl-D at an empty prompt), whereas + the sentinel with no text is merely an empty submission. """ console.print( styled( @@ -62,6 +68,7 @@ def read_multiline_lines(console: MarkupFreeConsole, sentinel: str = MULTILINE_S ) ) lines: list[str] = [] + hit_eof = False while True: try: line = input() @@ -69,11 +76,12 @@ def read_multiline_lines(console: MarkupFreeConsole, sentinel: str = MULTILINE_S # StopIteration only ever arises from a test double's exhausted # ``side_effect`` list (real ``input()`` never raises it) -- # treated the same as EOF: submit what has been accumulated. + hit_eof = True break if line.strip() == sentinel: break lines.append(line) - return "\n".join(lines).rstrip("\n") + return "\n".join(lines).rstrip("\n"), hit_eof async def read_on_daemon_thread[T](fn: Callable[[], T]) -> T: @@ -522,7 +530,8 @@ def _read_multiline(self) -> str: The collected text with trailing blank lines stripped. Internal newlines are preserved. """ - return read_multiline_lines(self.console, MULTILINE_SENTINEL) + text, _ = read_multiline_lines(self.console, MULTILINE_SENTINEL) + return text @dataclass diff --git a/tests/test_gates/test_dialog.py b/tests/test_gates/test_dialog.py index e45dadb1..00108009 100644 --- a/tests/test_gates/test_dialog.py +++ b/tests/test_gates/test_dialog.py @@ -896,6 +896,55 @@ async def test_eof_mid_paste_submits_content_not_dismissal(self) -> None: assert user_msgs[0].content == "ticket text" # not dropped as None provider.execute_dialog_turn.assert_awaited() # the turn WAS dispatched + @pytest.mark.asyncio + async def test_ctrl_d_at_empty_prompt_dismisses(self) -> None: + """A deliberate Ctrl-D with nothing typed ends the dialog. + + Regression: the multi-line reader converts EOF into a returned string, + so an empty read must not be fed back round the loop -- otherwise the + dismissal branch is unreachable and the grill cannot be exited. + """ + handler = DialogHandler(console=MagicMock()) + agent = AgentDef(name="t", prompt="p", dialog=DialogConfig(trigger_prompt="t")) + provider = MagicMock() + provider.execute_dialog_turn = AsyncMock(return_value="ack") + # A bounded list: if the loop spins, input() raises StopIteration + # rather than hanging the suite. + with ( + patch.object(handler, "_ask_engagement", new_callable=AsyncMock, return_value="engage"), + patch("conductor.gates.dialog.sys.stdin.isatty", return_value=True), + patch("builtins.input", side_effect=EOFError()), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + assert result.user_dismissed is True + assert [m for m in result.messages if m.role == "user"] == [] + provider.execute_dialog_turn.assert_not_awaited() + + @pytest.mark.asyncio + async def test_ctrl_c_dismisses_rather_than_propagating(self) -> None: + """KeyboardInterrupt on the tty turn dismisses, as on the single-line path.""" + handler = DialogHandler(console=MagicMock()) + agent = AgentDef(name="t", prompt="p", dialog=DialogConfig(trigger_prompt="t")) + provider = MagicMock() + provider.execute_dialog_turn = AsyncMock(return_value="ack") + with ( + patch.object(handler, "_ask_engagement", new_callable=AsyncMock, return_value="engage"), + patch("conductor.gates.dialog.sys.stdin.isatty", return_value=True), + patch("builtins.input", side_effect=KeyboardInterrupt()), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + assert result.user_dismissed is True + @pytest.mark.asyncio async def test_web_path_unaffected_by_multiline(self) -> None: """The web seam still passes whole messages through, never via the new reader.""" diff --git a/tests/test_gates/test_human.py b/tests/test_gates/test_human.py index 6665b049..896421ac 100644 --- a/tests/test_gates/test_human.py +++ b/tests/test_gates/test_human.py @@ -763,20 +763,37 @@ def test_read_multiline_lines_preserves_internal_newlines(self) -> None: "builtins.input", side_effect=["line one", "line two", "line three", "."], ): - result = read_multiline_lines(MagicMock()) + result, hit_eof = read_multiline_lines(MagicMock()) assert result == "line one\nline two\nline three" + assert hit_eof is False def test_read_multiline_lines_custom_sentinel(self) -> None: """A lone '.' is not a submit under a custom sentinel (AC3).""" with patch("builtins.input", side_effect=[".", "still going", "/send"]): - result = read_multiline_lines(MagicMock(), sentinel="/send") + result, hit_eof = read_multiline_lines(MagicMock(), sentinel="/send") assert result == ".\nstill going" + assert hit_eof is False def test_read_multiline_lines_eof_returns_accumulated(self) -> None: """EOF submits accumulated content, not empty (feeds AC4).""" with patch("builtins.input", side_effect=["a", "b", EOFError()]): - result = read_multiline_lines(MagicMock()) + result, hit_eof = read_multiline_lines(MagicMock()) assert result == "a\nb" + assert hit_eof is True + + def test_read_multiline_lines_reports_eof_on_empty_read(self) -> None: + """A bare EOF is distinguishable from an empty sentinel submission. + + The dialog gate relies on this to tell a deliberate Ctrl-D (dismiss) + from a sentinel typed with nothing above it (not a turn). + """ + with patch("builtins.input", side_effect=EOFError()): + text, hit_eof = read_multiline_lines(MagicMock()) + assert (text, hit_eof) == ("", True) + + with patch("builtins.input", side_effect=["."]): + text, hit_eof = read_multiline_lines(MagicMock()) + assert (text, hit_eof) == ("", False) From 84b9777af88ee3dc54f4352975ac7a9a50e45343 Mon Sep 17 00:00:00 2001 From: Chris Throup Date: Thu, 3 Sep 2026 10:47:03 +0200 Subject: [PATCH 3/4] PDA-35: advertise the /send sentinel in the Dialog Mode banner The multi-line turn requires /send to submit, but the opening banner still told users only to "type your responses below" and to say done//done when finished -- so the first instruction a user reads omitted the one keystroke a turn now needs. The per-turn hint from read_multiline_lines did name it, but the banner is read first. The sentinel is passed to styled() as a positional argument alongside agent.name, so the banner cannot drift from DIALOG_SUBMIT_SENTINEL. The new test renders the panel for real and asserts against the constant rather than a literal, matching the approach in tests/test_cli/test_markup_injection.py, which renders this same banner to catch interpolation that a mocked Panel would hide. Verified: tests/test_gates and tests/test_cli/test_markup_injection.py 116 passed; make lint clean. Rendered at width 76 to confirm the wrapped text reads correctly. Addresses the non-blocking documentation finding carried across both reviews on PR #5. --- src/conductor/gates/dialog.py | 7 +++++-- tests/test_gates/test_dialog.py | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/conductor/gates/dialog.py b/src/conductor/gates/dialog.py index c709a53e..11f21378 100644 --- a/src/conductor/gates/dialog.py +++ b/src/conductor/gates/dialog.py @@ -645,9 +645,12 @@ def _display_dialog_start( styled( "[bold]Agent '{}'[/bold] would like to discuss " "its output with you.\n" - "[dim]Type your responses below. Say [bold]done[/bold] or " - "[bold]/done[/bold] when finished.[/dim]", + "[dim]Type your response below; it can span multiple " + "lines. Send it with [bold]{}[/bold] on its own line. " + "Say [bold]done[/bold] or [bold]/done[/bold] when " + "finished.[/dim]", agent.name, + DIALOG_SUBMIT_SENTINEL, ), title=Text.from_markup("[bold magenta]Dialog Mode[/bold magenta]"), border_style="magenta", diff --git a/tests/test_gates/test_dialog.py b/tests/test_gates/test_dialog.py index 00108009..d1f79066 100644 --- a/tests/test_gates/test_dialog.py +++ b/tests/test_gates/test_dialog.py @@ -10,6 +10,7 @@ from conductor.config.schema import AgentDef, DialogConfig from conductor.gates.dialog import DialogHandler, DialogResult +from conductor.gates.human import DIALOG_SUBMIT_SENTINEL class TestDialogHandlerSkip: @@ -896,6 +897,25 @@ async def test_eof_mid_paste_submits_content_not_dismissal(self) -> None: assert user_msgs[0].content == "ticket text" # not dropped as None provider.execute_dialog_turn.assert_awaited() # the turn WAS dispatched + def test_opening_banner_advertises_the_submit_sentinel(self) -> None: + """The banner names the sentinel a turn actually requires. + + Rendered for real, and asserted against the constant rather than a + literal, so the banner cannot drift from DIALOG_SUBMIT_SENTINEL. + """ + import io + + from conductor.console import make_console + + buf = io.StringIO() + handler = DialogHandler(console=make_console(file=buf, width=300, no_color=True)) + agent = AgentDef(name="t", prompt="p", dialog=DialogConfig(trigger_prompt="t")) + + handler._display_dialog_start(agent, {"out": 1}, "question?") + + rendered = "".join(buf.getvalue().split()) + assert "".join(DIALOG_SUBMIT_SENTINEL.split()) in rendered, rendered + @pytest.mark.asyncio async def test_ctrl_d_at_empty_prompt_dismisses(self) -> None: """A deliberate Ctrl-D with nothing typed ends the dialog. From 2b00340d60cb0b0830b698b5c80b9ae10661ae38 Mon Sep 17 00:00:00 2001 From: Chris Throup Date: Thu, 3 Sep 2026 11:10:38 +0200 Subject: [PATCH 4/4] PDA-35: only advertise /send where the sentinel actually applies 84b9777 added the /send hint to the Dialog Mode banner unconditionally, but _display_dialog_start is called for every terminal dialog (dialog.py:248) while the multi-line reader is gated on sys.stdin.isatty(). Off a tty the turn falls back to the single-line Prompt.ask branch, where the sentinel does nothing -- so the banner told the user to type something with no effect, and it was the only instruction shown there (read_multiline_lines' per-turn hint is correctly absent). This was a regression introduced by 84b9777: on main the banner named only done//done, which is accurate on both paths. The hint is now built behind the same sys.stdin.isatty() condition as the reader, with the [bold] markup kept in the styled() template -- styled() inserts *values* verbatim precisely so they cannot be parsed as markup, so interpolating pre-marked-up text would render a literal "[bold]". The test is now parametrised over isatty and asserts presence-iff-tty rather than mere presence, so neither direction can regress unnoticed; it also asserts no literal "[bold]" survives. Confirmed it fails when the gate is removed (the isatty=False case) and passes with it. Low impact -- the realistic non-tty consumer is the web dashboard, which returns via _web_handle_dialog (dialog.py:217) and never renders this banner -- but the gate costs three lines and the condition already existed. Verified: tests/test_gates and tests/test_cli/test_markup_injection.py 117 passed; make lint clean; rendered both paths at width 76. Separately confirmed this panel emits no bold ANSI even on main, so the flattening of nested [bold] inside [dim] is pre-existing and not introduced here. Addresses the non-blocking finding from the third review on PR #5. --- src/conductor/gates/dialog.py | 19 ++++++++++++++++--- tests/test_gates/test_dialog.py | 17 +++++++++++++---- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/conductor/gates/dialog.py b/src/conductor/gates/dialog.py index 11f21378..ae7af75f 100644 --- a/src/conductor/gates/dialog.py +++ b/src/conductor/gates/dialog.py @@ -639,18 +639,31 @@ def _display_dialog_start( base_dir: Path | None = None, ) -> None: """Display the dialog opening with full agent context.""" + # Gated on the same condition as the multi-line reader in + # _get_user_input: off a tty, that turn falls back to a single-line + # Prompt.ask and the sentinel does nothing, so advertising it would + # instruct the user to type something with no effect. The markup stays + # in the template because styled() inserts *values* verbatim. + if sys.stdin.isatty(): + multiline_hint = ( + " It can span multiple lines; send it with [bold]{}[/bold] on its own line." + ) + hint_args: tuple[object, ...] = (DIALOG_SUBMIT_SENTINEL,) + else: + multiline_hint = "" + hint_args = () + self.console.print() self.console.print( Panel( styled( "[bold]Agent '{}'[/bold] would like to discuss " "its output with you.\n" - "[dim]Type your response below; it can span multiple " - "lines. Send it with [bold]{}[/bold] on its own line. " + "[dim]Type your response below." + multiline_hint + " " "Say [bold]done[/bold] or [bold]/done[/bold] when " "finished.[/dim]", agent.name, - DIALOG_SUBMIT_SENTINEL, + *hint_args, ), title=Text.from_markup("[bold magenta]Dialog Mode[/bold magenta]"), border_style="magenta", diff --git a/tests/test_gates/test_dialog.py b/tests/test_gates/test_dialog.py index d1f79066..2cbc93e1 100644 --- a/tests/test_gates/test_dialog.py +++ b/tests/test_gates/test_dialog.py @@ -897,8 +897,13 @@ async def test_eof_mid_paste_submits_content_not_dismissal(self) -> None: assert user_msgs[0].content == "ticket text" # not dropped as None provider.execute_dialog_turn.assert_awaited() # the turn WAS dispatched - def test_opening_banner_advertises_the_submit_sentinel(self) -> None: - """The banner names the sentinel a turn actually requires. + @pytest.mark.parametrize("isatty", [True, False]) + def test_opening_banner_advertises_the_sentinel_only_on_a_tty(self, isatty: bool) -> None: + """The banner names the sentinel exactly when a turn requires it. + + Off a tty the turn falls back to the single-line ``Prompt.ask`` branch, + where the sentinel does nothing -- so advertising it there would tell + the user to type something with no effect. Rendered for real, and asserted against the constant rather than a literal, so the banner cannot drift from DIALOG_SUBMIT_SENTINEL. @@ -911,10 +916,14 @@ def test_opening_banner_advertises_the_submit_sentinel(self) -> None: handler = DialogHandler(console=make_console(file=buf, width=300, no_color=True)) agent = AgentDef(name="t", prompt="p", dialog=DialogConfig(trigger_prompt="t")) - handler._display_dialog_start(agent, {"out": 1}, "question?") + with patch("conductor.gates.dialog.sys.stdin.isatty", return_value=isatty): + handler._display_dialog_start(agent, {"out": 1}, "question?") rendered = "".join(buf.getvalue().split()) - assert "".join(DIALOG_SUBMIT_SENTINEL.split()) in rendered, rendered + needle = "".join(DIALOG_SUBMIT_SENTINEL.split()) + assert (needle in rendered) is isatty, rendered + # The markup must be parsed, not inserted verbatim as a value. + assert "[bold]" not in rendered, rendered @pytest.mark.asyncio async def test_ctrl_d_at_empty_prompt_dismisses(self) -> None: