diff --git a/AGENTS.md b/AGENTS.md index 9710bcd0..cfae2a37 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -158,7 +158,8 @@ step-by-step checklist. - `factory.py` - Provider instantiation - **gates/**: Human-in-the-loop support - - `human.py` - Rich terminal UI for human gate interactions + - `human.py` - Rich terminal UI for human gate interactions. `read_multiline_lines(console, sentinel)` is the shared blocking multi-line stdin reader behind both the human gate's `.` sentinel (`MULTILINE_SENTINEL`) and the dialog gate's `/send` (`DIALOG_SUBMIT_SENTINEL`); it returns `(text, hit_eof)` because an EOF that yielded no text is a deliberate dismissal while the sentinel with no text is an empty submission. `sentinel` is keyword-only and required, since the two gates use different ones + - `dialog.py` - Dialog-mode gate. On a tty the main turn reads through `read_multiline_lines`, so a pasted block is one turn rather than one turn per line; off a tty it falls back to single-line `Prompt.ask`, where the sentinel has no effect — the `_reads_multiline_turn()` predicate states that rule once, and `_display_dialog_start` gates its sentinel hint on it so the banner never advertises a keystroke the reader ignores. Both submission guards compare against stripped text: the reader drops trailing newlines but keeps a whitespace-only line, so an exact `== ""` check would dispatch whitespace as a turn and turn Ctrl-D into a submission - **interrupt/**: Interactive workflow interruption (Esc/Ctrl+G to pause) - `listener.py` - Keyboard listener daemon thread for Esc/Ctrl+G detection diff --git a/CHANGELOG.md b/CHANGELOG.md index 9528fcf8..866b5e90 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,35 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 (`reason: "estimate_unavailable"`) rather than vanishing into stderr. See [Workflow Syntax → Context Compaction](docs/workflow-syntax.md#context-compaction). +- **A multi-line reply to a terminal dialog is now one turn** (#509) — + dialog mode was the only free-text human-input surface that could not accept + a multi-line answer (`QuestionDef.multiline` defaults to `True` and + `GateOption.multiline` opts in, both served by one reader in `gates/human.py`). + The dialog gate read a reply with single-line `Prompt.ask`, so pasting a block + of text into an interactive terminal dispatched *each line* as its own turn: a + three-line paste became three separate questions to the model, each answered + against a fragment, and the paste's trailing newline added a fourth turn with + empty content. Terminal turns now read through the multi-line reader already + behind the human gate's `.` sentinel, submitted with `/send` on its own line, + so internal newlines survive and a paste is a single message. An empty or + whitespace-only submission is no longer dispatched as a turn. A dismiss + keyword is recognised only once a turn is submitted, so on a tty `done` now + needs `/send` after it, and both the opening banner and the failure-recovery + notice say so rather than naming a keystroke that does nothing there. + + Ctrl-D at the start of a line (Ctrl-Z then Enter on Windows) also submits the + lines entered so far, or dismisses the dialog when there are none. Because a + terminal's EOF does not persist, abandoning a part-written reply that way now + sends what was already entered and a second Ctrl-D is needed to leave, where + one used to exit; on an empty prompt it still exits in one keystroke. + + The dialog uses `/send` where the human gate keeps `.`, since a lone `.` is + likelier to be prose in a conversational reply. Off a tty — a pipe or CI — + replies are still read one line at a time and `/send` has no effect; the one + change on that path is that a blank line is now skipped instead of dispatched + as an empty turn. The web dashboard is unaffected: it takes a separate path + that already delivered each message whole. + ### Changed - **A `working_dir` or `settings_dir` template that renders empty is now an diff --git a/docs/workflow-syntax.md b/docs/workflow-syntax.md index a7763744..5bbb6b8e 100644 --- a/docs/workflow-syntax.md +++ b/docs/workflow-syntax.md @@ -1320,6 +1320,7 @@ After the conversation, the agent re-executes with the dialog transcript as addi **Behavior notes:** - Dialog is supported on regular `agent` type only (not `human_gate`, `questions`, `script`, `workflow`, or `wait`) +- In an interactive terminal, a reply may span multiple lines — paste or type freely and submit the turn with `/send` on its own line. A dismiss keyword ends the dialog the same way, so `done` needs `/send` after it there; off a tty every line is already a turn, so it does not. Ctrl-D at the start of a line (Ctrl-Z then Enter on Windows) also submits whatever lines have been entered so far, or dismisses the dialog when none have — so abandoning a part-written reply that way sends the lines already entered; press it on an empty prompt to leave without sending. An empty or whitespace-only submission is skipped rather than sent. Off a tty (a pipe or CI) replies are read one line at a time and `/send` does not apply, though a blank line is skipped rather than sent as an empty turn. The web dashboard is unaffected — its chat box takes a separate path that has always delivered each message whole, multi-line included - In web dashboard mode, the dialog temporarily replaces the graph area with a chat interface - When `--skip-gates` is set (e.g., CI/automation), dialogs are automatically skipped - The evaluator prompt should describe *when* to trigger dialog, not *what* to ask — the evaluator generates the opening question from the agent's output context diff --git a/src/conductor/gates/dialog.py b/src/conductor/gates/dialog.py index b38f88a5..acc8e820 100644 --- a/src/conductor/gates/dialog.py +++ b/src/conductor/gates/dialog.py @@ -11,6 +11,7 @@ import asyncio import json import logging +import sys import uuid from dataclasses import dataclass, field from typing import TYPE_CHECKING, Any @@ -22,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 @@ -81,6 +87,38 @@ _READY_MARKER = "[READY_TO_CONTINUE]" +def _reads_multiline_turn() -> bool: + """Whether a conversational dialog turn is read multi-line. + + Off a tty the turn falls back to a single-line ``Prompt.ask``, where the + submit sentinel has no effect. Both the reader in + ``DialogHandler._get_user_input`` and the opening banner's sentinel hint + ask this, so the banner can never advertise a keystroke the reader ignores. + """ + return sys.stdin.isatty() + + +def _dismiss_instruction() -> Text: + """How to actually leave the dialog, phrased for the active reader. + + A dismiss keyword is only recognised once the turn is *submitted*, so on a + tty it needs the sentinel after it. Saying "type done" there describes a + keystroke that does nothing until the reply is sent -- the exit instruction + has to move with :func:`_reads_multiline_turn` or it contradicts the very + banner that advertises the sentinel. + + Returns: + A pre-styled ``Text``, so a caller splices it through ``styled`` and + keeps its own template a literal (#406). + """ + if _reads_multiline_turn(): + return styled( + "send [bold]done[/bold] or [bold]/done[/bold] with [bold]{}[/bold]", + DIALOG_SUBMIT_SENTINEL, + ) + return Text.from_markup("say [bold]done[/bold] or [bold]/done[/bold]") + + def _extract_ready_marker(response: str) -> tuple[bool, str]: """Return ``(proposed, cleaned)`` for an agent response. @@ -273,6 +311,12 @@ async def handle_dialog( result.user_dismissed = True break + if not user_input.strip(): + # Empty submission -- not a turn, and not dismissal either. On + # a tty this is a bare sentinel line; off a tty it is a blank + # line from the pipe, which ``Prompt.ask`` returns as "". + continue + result.messages.append(DialogMessage(role="user", content=user_input)) self._emit_event( "dialog_message", @@ -309,9 +353,9 @@ async def handle_dialog( exc_info=True, ) self.console.print( - Text.from_markup( - "[dim red] (Agent response failed — you can continue " - "or type 'done')[/dim red]" + styled( + "[dim red] (Agent response failed — you can continue, or {})[/dim red]", + _dismiss_instruction(), ) ) continue @@ -600,15 +644,40 @@ def _display_dialog_start( base_dir: Path | None = None, ) -> None: """Display the dialog opening with full agent context.""" + # Advertised only where the sentinel applies -- see + # _reads_multiline_turn. Pre-rendered as a Text so the outer template + # has fixed arity: styled() splices a Text in with its own spans + # re-anchored, and raises IndexError on a template/argument mismatch + # that only the tty branch would reach. Off a tty the sentence keeps + # its original plural, since each line really is a separate response + # there. + instruction = ( + styled( + "Type your response below. It can span multiple lines; send it" + " with [bold]{}[/bold] on its own line. A dismiss keyword ends" + " the dialog the same way: send [bold]done[/bold] or" + " [bold]/done[/bold] with [bold]{}[/bold].", + DIALOG_SUBMIT_SENTINEL, + DIALOG_SUBMIT_SENTINEL, + ) + if _reads_multiline_turn() + # Byte-identical to upstream's sentence: off a tty every line is a + # turn already, so a dismiss keyword needs nothing after it. + else Text.from_markup( + "Type your responses below. Say [bold]done[/bold] or " + "[bold]/done[/bold] when finished." + ) + ) + self.console.print() self.console.print( Panel( 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]{}[/dim]", agent.name, + instruction, ), title=Text.from_markup("[bold magenta]Dialog Mode[/bold magenta]"), border_style="magenta", @@ -732,8 +801,26 @@ 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 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 + but whitespace accumulated is a deliberate Ctrl-D and still + returns None. """ + if prompt_text is None and _reads_multiline_turn(): + self.console.print(styled("[bold magenta]You[/bold magenta]")) + try: + text, hit_eof = await read_on_daemon_thread( + lambda: read_multiline_lines(self.console, sentinel=DIALOG_SUBMIT_SENTINEL) + ) + except (EOFError, KeyboardInterrupt): + return None + if hit_eof and not text.strip(): + # 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 99fab198..8e9cbf35 100644 --- a/src/conductor/gates/human.py +++ b/src/conductor/gates/human.py @@ -32,12 +32,68 @@ 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. + +Distinct from :data:`MULTILINE_SENTINEL` because a lone ``.`` is likelier to +be an ordinary line of prose in a conversational reply than in a gate answer. +""" + 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) -> 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. + Internal newlines are preserved; trailing empty lines are dropped, but a + trailing line of whitespace is kept verbatim -- a real strip would eat the + meaningful indentation of a pasted code block. + + Args: + console: Console to print the input hint to. + sentinel: Line that, typed alone, submits the accumulated text. + Keyword-only and required: the two gates use different sentinels, + so a caller states which one it means. + + Returns: + ``(text, hit_eof)`` -- the collected text 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( + " [dim]Enter your answer. Finish with '{}' on its own line (or {}).[/dim]", + sentinel, + _eof_key_hint(), + ) + ) + lines: list[str] = [] + hit_eof = False + while True: + try: + line = input() + except EOFError: + # The only end-of-input a real stdin produces here: an exhausted + # or non-tty stream raises EOFError, a closed one ValueError. + # StopIteration is deliberately *not* caught -- input() would only + # relay it from a contrived stdin replacement, and treating it as + # EOF would submit a truncated turn as if the user had pressed + # Ctrl-D. A test double that runs past what it supplied is a bug + # in the test, so it must surface rather than read as a dismissal. + hit_eof = True + break + if line.strip() == sentinel: + break + lines.append(line) + return "\n".join(lines).rstrip("\n"), hit_eof + + 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 +533,15 @@ 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. Returns: - The collected text with trailing blank lines stripped. Internal - newlines are preserved. + The collected text, with trailing empty lines dropped and internal + newlines 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") + text, _ = read_multiline_lines(self.console, sentinel=MULTILINE_SENTINEL) + return text @dataclass diff --git a/tests/test_gates/test_dialog.py b/tests/test_gates/test_dialog.py index 4b89e8c6..9e2c8c27 100644 --- a/tests/test_gates/test_dialog.py +++ b/tests/test_gates/test_dialog.py @@ -2,13 +2,20 @@ from __future__ import annotations +import itertools from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import pytest from conductor.config.schema import AgentDef, DialogConfig +from conductor.console import styled from conductor.gates.dialog import DialogHandler, DialogResult +from conductor.gates.human import ( + DIALOG_SUBMIT_SENTINEL, + read_multiline_lines, + read_on_daemon_thread, +) class TestDialogHandlerSkip: @@ -728,3 +735,444 @@ 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.""" + + @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", EOFError()], + ), + ): + 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", EOFError()]), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + # The paste-terminating EOF submitted its content and did NOT dismiss -- + # the dialog went on to accept a further turn, which is what ended it. + assert [m.content for m in result.messages if m.role == "user"] == [ + "ticket text", + "done", + ] + provider.execute_dialog_turn.assert_awaited_once() + + @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. + """ + 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")) + + with patch("conductor.gates.dialog.sys.stdin.isatty", return_value=isatty): + handler._display_dialog_start(agent, {"out": 1}, "question?") + + rendered = "".join(buf.getvalue().split()) + 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 + # Off a tty the sentence keeps its original plural, since each line + # really is a separate response there. Asserted because this wording + # has already drifted to the singular once, and only a byte comparison + # against the unmodified banner would otherwise have caught it. + expected = "Typeyourresponsesbelow." if not isatty else "Typeyourresponsebelow." + assert expected in rendered, rendered + # The exit instruction has to describe the *active* reader. On a tty a + # dismiss keyword is only seen once the turn is submitted, so telling + # the user to "say done" there names a keystroke that does nothing. + if isatty: + assert "senddoneor/donewith/send" in rendered, rendered + assert "Saydoneor/donewhenfinished." not in rendered, rendered + else: + assert "Saydoneor/donewhenfinished." 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. + + 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 dialog 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* EOF source. ``side_effect=EOFError()`` re-raises forever, + # so dropping the dismissal branch would spin this loop and hang the + # suite rather than fail it -- there is no pytest-timeout configured. + calls = itertools.count() + + def _eof_but_bounded(*_args: object, **_kwargs: object) -> str: + assert next(calls) < 10, "dialog loop spun on an empty EOF read" + raise EOFError + + 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=_eof_but_bounded), + patch( + "conductor.gates.dialog.read_multiline_lines", + wraps=read_multiline_lines, + ) as reader, + patch( + "conductor.gates.dialog.read_on_daemon_thread", + wraps=read_on_daemon_thread, + ) as dispatch, + ): + 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() + # Pins *this* reader, not merely "some reader dismissed on EOF". + reader.assert_called_once() + # And pins the dispatch: a cancelled ``asyncio.to_thread`` leaves its + # worker blocked in ``input()`` holding a slot in the shared default + # executor, which eventually deadlocks unrelated ``to_thread`` calls -- + # see ``read_on_daemon_thread``'s own docstring. + dispatch.assert_called_once() + + @pytest.mark.asyncio + async def test_reader_exception_dismisses_rather_than_crashing_the_dialog(self) -> None: + """An exception out of the reader dismisses instead of escaping. + + This does **not** cover Ctrl-C. CPython runs signal handlers on the + main thread only, and the read happens on a daemon thread, so a real + SIGINT never reaches this ``except``: asyncio cancels the main task and + ``KeyboardInterrupt`` tears the run down, as it does everywhere else. + What is covered is an exception the reader itself raises. + """ + 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.""" + 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 + + @pytest.mark.asyncio + async def test_non_tty_main_turn_uses_the_single_line_prompt(self) -> None: + """Off a tty the conversational turn stays on ``Prompt.ask``. + + Half of the reader gate: without the ``isatty()`` check the multi-line + reader would activate under a pipe or in CI, waiting for a sentinel + nobody can type. + """ + 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=False), + patch( + "conductor.gates.dialog.Prompt.ask", + side_effect=["piped answer", "done"], + ) as ask, + patch("builtins.input", side_effect=AssertionError("must not read raw stdin")), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + assert ask.call_count == 2 + assert [m.content for m in result.messages if m.role == "user"] == [ + "piped answer", + "done", + ] + + @pytest.mark.asyncio + async def test_confirmation_prompt_stays_single_line_on_a_tty(self) -> None: + """A ``prompt_text`` question must not require the sentinel. + + The other half of the reader gate: without the ``prompt_text is None`` + check the yes/no confirmation would start demanding ``/send`` after + "yes" on every interactive run. + """ + handler = DialogHandler(console=MagicMock()) + with ( + patch("conductor.gates.dialog.sys.stdin.isatty", return_value=True), + patch("conductor.gates.dialog.Prompt.ask", return_value="yes") as ask, + patch("builtins.input", side_effect=AssertionError("must not read multi-line")), + ): + answer = await handler._get_user_input(prompt_text=styled("[bold]Continue?[/bold]")) + assert answer == "yes" + ask.assert_called_once() + + @pytest.mark.asyncio + async def test_bare_sentinel_is_skipped_and_the_loop_continues(self) -> None: + """An empty submission is neither a turn nor 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), + patch( + "builtins.input", + side_effect=["/send", "real turn", "/send", "done", "/send", EOFError()], + ), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + # No empty turn recorded, none dispatched, and the loop carried on to + # accept a real turn afterwards. + assert [m.content for m in result.messages if m.role == "user"] == [ + "real turn", + "done", + ] + provider.execute_dialog_turn.assert_awaited_once() + assert result.user_dismissed is True + + @pytest.mark.parametrize("isatty", [True, False]) + def test_failure_notice_names_a_working_exit(self, isatty: bool) -> None: + """The recovery notice must not name an inert keystroke either. + + It fires when a provider call has just failed -- the moment the user + most wants a reliable way out -- so it has to move with the reader the + same way the banner does. + """ + from conductor.gates.dialog import _dismiss_instruction + + with patch("conductor.gates.dialog.sys.stdin.isatty", return_value=isatty): + hint = _dismiss_instruction() + + assert ("with /send" in hint) is isatty, hint + + @pytest.mark.asyncio + async def test_dismiss_keyword_still_exits_a_tty_dialog(self) -> None: + """ "done" submitted with the sentinel ends the dialog. + + The banner promises this; without it the only exits from a tty dialog + would be Ctrl-D and whatever the agent decides. + """ + 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=["done", "/send", EOFError()]), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + assert result.user_dismissed is True + provider.execute_dialog_turn.assert_not_awaited() + + @pytest.mark.asyncio + async def test_pasted_indentation_reaches_the_provider_intact(self) -> None: + """A pasted code block keeps its leading and interior whitespace. + + The empty-submission guards strip only to *decide* whether there is + anything to send; the text itself must go through verbatim. Stripping + it would silently reindent a pasted block, which is the data loss this + whole reader exists to prevent, and no other test covers the leading + edge of it. + """ + 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") + block = [" def f():", "", " return 1", " "] + 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=[*block, "/send", "done", EOFError()]), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + expected = " def f():\n\n return 1\n " + assert [m.content for m in result.messages if m.role == "user"][0] == expected + assert provider.execute_dialog_turn.await_args.kwargs["user_message"] == expected + + @pytest.mark.asyncio + async def test_whitespace_only_submission_is_not_a_turn(self) -> None: + """Whitespace must not slip past the empty-submission guard. + + The reader strips trailing newlines, not whitespace, so a buffer of + spaces survives as a truthy string. Dispatched, it would re-run the + agent believing the user replied with whitespace. + """ + 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=[" ", "/send", "done", EOFError()]), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + assert [m.content for m in result.messages if m.role == "user"] == ["done"] + provider.execute_dialog_turn.assert_not_awaited() + + @pytest.mark.asyncio + async def test_ctrl_d_after_whitespace_dismisses_without_submitting(self) -> None: + """Ctrl-D means "I am leaving", even with whitespace in the buffer. + + Without a stripping guard the whitespace is dispatched as a turn and + the dialog dismisses afterwards -- the user asked to leave and sent a + message instead. + """ + 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=[" ", EOFError()]), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + provider.execute_dialog_turn.assert_not_awaited() + assert [m for m in result.messages if m.role == "user"] == [] + assert result.user_dismissed is True + + @pytest.mark.asyncio + @pytest.mark.parametrize("blank", ["", " "]) + async def test_blank_piped_line_is_skipped_off_a_tty(self, blank: str) -> None: + """The empty guard also covers the non-tty path. + + ``Prompt.ask`` returns "" for a blank line, which previously reached + the provider as an empty turn. The whitespace case is parametrised + because ``rich.prompt.PromptBase.process_response`` strips its result, + so in production a whitespace-only line already arrives as "" -- these + mocks bypass that, and the guard has to hold either way. + """ + 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=False), + patch("conductor.gates.dialog.Prompt.ask", side_effect=[blank, "done"]), + ): + result = await handler.handle_dialog( + agent=agent, + agent_output={"result": "x"}, + opening_question="Q?", + provider=provider, + ) + assert [m.content for m in result.messages if m.role == "user"] == ["done"] + provider.execute_dialog_turn.assert_not_awaited() diff --git a/tests/test_gates/test_human.py b/tests/test_gates/test_human.py index c198239c..baf7d968 100644 --- a/tests/test_gates/test_human.py +++ b/tests/test_gates/test_human.py @@ -9,7 +9,13 @@ from conductor.config.schema import AgentDef, GateOption from conductor.exceptions import HumanGateError -from conductor.gates.human import GateResult, HumanGateHandler +from conductor.gates.human import ( + DIALOG_SUBMIT_SENTINEL, + MULTILINE_SENTINEL, + GateResult, + HumanGateHandler, + read_multiline_lines, +) @pytest.fixture @@ -752,3 +758,117 @@ 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, hit_eof = read_multiline_lines(MagicMock(), sentinel=MULTILINE_SENTINEL) + + 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.""" + with patch("builtins.input", side_effect=[".", "still going", "/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.""" + with patch("builtins.input", side_effect=["a", "b", EOFError()]): + result, hit_eof = read_multiline_lines(MagicMock(), sentinel=MULTILINE_SENTINEL) + + 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(), sentinel=MULTILINE_SENTINEL) + assert (text, hit_eof) == ("", True) + + with patch("builtins.input", side_effect=["."]): + text, hit_eof = read_multiline_lines(MagicMock(), sentinel=MULTILINE_SENTINEL) + assert (text, hit_eof) == ("", False) + + @pytest.mark.parametrize("sentinel", [MULTILINE_SENTINEL, DIALOG_SUBMIT_SENTINEL]) + def test_hint_names_the_sentinel_it_will_accept(self, sentinel: str) -> None: + """The per-turn hint is where the user learns how to submit. + + Each gate passes its own sentinel, so a hint rendered from anything + else would tell the user to type a line the reader ignores -- and + leave them at a prompt that never submits. + """ + console = MagicMock() + with patch("builtins.input", side_effect=[sentinel]): + read_multiline_lines(console, sentinel=sentinel) + + printed = console.print.call_args.args[0].plain + assert f"'{sentinel}'" in printed, printed + + def test_a_broken_stdin_is_not_read_as_a_dismissal(self) -> None: + """Only EOFError ends the read; anything else propagates. + + A stdin source raising something the loop swallowed would submit a + truncated turn as though the user had pressed Ctrl-D, with nothing + logged. StopIteration is the case that matters: it is what an + exhausted test double raises, so catching it would also let a double + read past what it supplied and still pass as a clean submission. + """ + with ( + patch("builtins.input", side_effect=StopIteration("broken source")), + pytest.raises(StopIteration), + ): + read_multiline_lines(MagicMock(), sentinel=MULTILINE_SENTINEL) + + def test_sentinel_must_be_passed_by_keyword(self) -> None: + """The sentinel is required, so neither gate can inherit the other's. + + A positional default made ``read_multiline_lines(console)`` silently + valid, which would truncate a dialog reply at any lone "." with no + signal at the call site. Pinned because the hazard is re-openable by + restoring one default and nothing else would fail. + """ + with pytest.raises(TypeError, match="sentinel"): + read_multiline_lines(MagicMock()) # ty: ignore[missing-argument] + + @pytest.mark.parametrize("typed", ["/send", " /send", "/send ", "\t/send "]) + def test_sentinel_tolerates_surrounding_whitespace(self, typed: str) -> None: + """A stray space around the sentinel still submits. + + Terminals and paste buffers add trailing whitespace routinely, and + without this the user would sit at a prompt that never submits. The + trailing ``"unreachable"`` proves the reader stopped at the sentinel + rather than merely running out of mock values. + """ + with patch("builtins.input", side_effect=["body", typed, "unreachable"]): + text, hit_eof = read_multiline_lines(MagicMock(), sentinel="/send") + + assert (text, hit_eof) == ("body", False) + + def test_trailing_whitespace_line_is_kept_verbatim(self) -> None: + """Trailing *empty* lines are dropped; a whitespace line is content. + + Pins the docstring's distinction: stripping it would eat the closing + indentation of a pasted code block. + """ + with patch("builtins.input", side_effect=["a", "", "", "/send"]): + text, _ = read_multiline_lines(MagicMock(), sentinel="/send") + assert text == "a" + + with patch("builtins.input", side_effect=["a", " ", "", "/send"]): + text, _ = read_multiline_lines(MagicMock(), sentinel="/send") + assert text == "a\n "