Skip to content
Open
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
33 changes: 31 additions & 2 deletions cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -13472,9 +13472,38 @@ def run_agent():
all_parts.append(extra)
except queue.Empty:
break
combined = "\n".join(all_parts)
# Parts are plain `str`, or `(text, [Path, ...])` tuples when
# the Enter handler bundled attached images. `"\n".join` raises
# TypeError on a tuple, and since the extras were already taken
# off the queue with get_nowait() — and chat()'s bare `except`
# swallows the error — the message and its images were silently
# lost. Merge type-aware instead: join the text, concatenate the
# image lists, and emit a plain `str` when no part carries
# images so the text-only path is unchanged. process_loop
# already unpacks the `(text, images)` form.
text_parts = []
merged_images = []
for part in all_parts:
if isinstance(part, tuple) and len(part) == 2:
part_text, part_images = part
else:
part_text, part_images = part, None
if part_text:
text_parts.append(
part_text if isinstance(part_text, str) else str(part_text)
)
if part_images:
merged_images.extend(part_images)
combined_text = "\n".join(text_parts)
combined = (combined_text, merged_images) if merged_images else combined_text
n = len(all_parts)
preview = combined[:50] + ("..." if len(combined) > 50 else "")
preview = combined_text[:50] + ("..." if len(combined_text) > 50 else "")
if merged_images:
_img_note = (
f"[{len(merged_images)} image"
f"{'s' if len(merged_images) != 1 else ''} attached]"
)
preview = f"{preview} {_img_note}" if preview else _img_note
if n > 1:
print(f"\n⚡ Sending {n} messages after interrupt: '{preview}'")
else:
Expand Down
122 changes: 122 additions & 0 deletions tests/cli/test_cli_interrupt_ack_race.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import threading
import time
import types
from pathlib import Path
from unittest.mock import MagicMock, patch


Expand Down Expand Up @@ -196,6 +197,127 @@ def run_conversation(self, **kwargs):
assert cli._last_turn_interrupted is True


class _AcknowledgingStubAgent(_StubAgent):
"""Agent whose turn result DOES acknowledge the interrupt.

Mirrors the private ``_AckAgent`` above; shared by the image-payload
tests so both can drive the live ``chat()`` requeue branch.
"""

def run_conversation(self, **kwargs):
# Wait until the monitor loop delivers the interrupt.
for _ in range(100):
if self._interrupt_requested:
break
time.sleep(0.05)
return {
"final_response": "partial work",
"messages": [{"role": "assistant", "content": "partial work"}],
"api_calls": 1,
"completed": False,
"interrupted": True,
"interrupt_message": self._interrupt_message,
"partial": True,
"response_previewed": True,
}


def _run_interrupted_turn(cli):
"""Run one ``chat()`` turn with the runtime/credential plumbing stubbed."""
with patch.object(cli, "_ensure_runtime_credentials", return_value=True), \
patch.object(cli, "_resolve_turn_agent_config", return_value={
"signature": cli._active_agent_route_signature,
"model": None, "runtime": None, "request_overrides": None,
}), \
patch.object(cli, "_init_agent", return_value=True):
cli.chat("original")

queued = []
while not cli._pending_input.empty():
queued.append(cli._pending_input.get_nowait())
return queued


def test_interrupt_with_attached_images_is_requeued_intact():
"""A single image interrupt must survive the requeue (supersedes #5202).

The Enter handler bundles attached images as ``(text, [Path, ...])`` and
puts that tuple on ``_interrupt_queue``, so ``pending_message`` itself can
be a tuple — one interrupted image message is enough to trip the old
``"\\n".join(all_parts)``. The resulting ``TypeError`` was swallowed by
``chat()``'s bare ``except``, and because the parts had already been taken
off the queue the message and its images were gone for good.
"""
cli = _make_cli()
cli.agent = _AcknowledgingStubAgent(cli.session_id)
cli._interrupt_queue = queue.Queue()
cli._pending_input = queue.Queue()

shot = Path("/tmp/hermes-test-shot.png")
cli._interrupt_queue.put(("what is in this screenshot?", [shot]))

queued = _run_interrupted_turn(cli)

assert queued, "image interrupt payload was dropped instead of re-queued"
payload = queued[0]
assert isinstance(payload, tuple), (
f"image payload lost its attachments; pending_input={queued!r}"
)
text, images = payload
assert text == "what is in this screenshot?"
assert images == [shot]


def test_interrupt_merges_text_and_image_parts_without_losing_attachments():
"""Extras drained from ``_interrupt_queue`` merge type-aware.

The monitor loop claims the first queued item as the interrupt; anything
typed after it stays on ``_interrupt_queue`` and is drained by the requeue
branch. Text joins with newlines (unchanged behaviour) and every image list
is concatenated in arrival order.
"""
cli = _make_cli()
cli.agent = _AcknowledgingStubAgent(cli.session_id)
cli._interrupt_queue = queue.Queue()
cli._pending_input = queue.Queue()

first = Path("/tmp/hermes-test-first.png")
second = Path("/tmp/hermes-test-second.png")
cli._interrupt_queue.put(("look at this", [first]))
cli._interrupt_queue.put("wait, also stop the build")
cli._interrupt_queue.put(("and this one", [second]))

queued = _run_interrupted_turn(cli)

assert queued, "merged interrupt payload was dropped instead of re-queued"
payload = queued[0]
assert isinstance(payload, tuple), (
f"merged payload lost its attachments; pending_input={queued!r}"
)
text, images = payload
assert text == "look at this\nwait, also stop the build\nand this one"
assert images == [first, second]


def test_text_only_interrupt_still_requeues_a_plain_string():
"""No part carries images → the payload stays a plain ``str``.

Guards the common path: process_loop only unpacks tuples, so a text-only
interrupt must not start arriving as ``(text, [])``.
"""
cli = _make_cli()
cli.agent = _AcknowledgingStubAgent(cli.session_id)
cli._interrupt_queue = queue.Queue()
cli._pending_input = queue.Queue()

cli._interrupt_queue.put("stop")
cli._interrupt_queue.put("show me the plan instead")

queued = _run_interrupted_turn(cli)

assert queued == ["stop\nshow me the plan instead"]


def test_chat_persists_clean_input_when_a_queued_note_changes_api_message():
"""Queued notes remain API-local and preserve close-handoff marker identity."""
cli = _make_cli()
Expand Down
Loading