fix(cli): keep attached images when interrupted input is re-queued - #73104
fix(cli): keep attached images when interrupted input is re-queued#73104briandevans wants to merge 1 commit into
Conversation
Interrupting a running turn with images attached silently destroyed the message. The Enter handler bundles attachments as a `(text, [Path, ...])` tuple and, in the default `busy_input_mode == "interrupt"`, puts that tuple on `_interrupt_queue`. `HermesCLI.chat()`'s requeue branch then ran `"\n".join(all_parts)` over the drained parts, which raises `TypeError: sequence item 0: expected str instance, tuple found`. Because `pending_message` is itself bound from the queue value, a single interrupted image message is enough to trip it — no second message needed. The failure was invisible and unrecoverable: `chat()` runs under a bare `except Exception`, so the user only saw a one-line `Error: ...`, and the parts had already been removed from the queue with `get_nowait()`. The message and its images were gone — not requeued, not shown, not recallable. Merge the parts type-aware at the requeue site instead: join the text, concatenate the image lists, and emit a plain `str` when no part carries images so the common text-only path is byte-for-byte unchanged. `process_loop` already unpacks the `(text, images)` form, so the merged payload round-trips into the next turn as a real multimodal message. The preview string stays a `str` and gains an image count when attachments are present. Supersedes NousResearch#5202, which diagnosed the same defect in April but tested extracted helpers in isolation and never exercised the live `chat()` requeue branch.
alt-glitch
added
type/bug
Related: #5202 diagnoses the same image-interrupt loss, but this PR applies the repair and regression coverage at the current live |
|
Thanks for the focused fix. The premise is confirmed on current The added tests exercise the live Automated hermes-sweeper review. |
teknium1
added
sweeper:risk-message-delivery
3 participants
This supersedes #5202 (@mattsegura, 2026-04-05)
chat()interrupt-requeue path, and the credit for finding this belongs there.tests/test_cli_interrupt_payloads.pyexercises three extracted module-level helpers in isolation and never drives the liveHermesCLI.chat()requeue branch — the exact deficiency named in the 2026-07-12 review on that PR. It also carries an unrelatedprocess_loopdisplay refactor, and itscli.pyhunks are stale against acli.pythat has been rewritten around them in the 3.5 months since (the requeue branch moved from ~line 6666 to 13466; the tuple unpack it adds toprocess_loopis already inline on main atcli.py:16132-16134).tests/cli/test_cli_interrupt_ack_race.py, asserting the queued next-turn payload retains(text, [Path, ...]).What does this PR do?
Interrupting a running turn while images are attached silently destroys the message and the images.
The Enter handler bundles attachments as a
(text, [Path, ...])tuple (cli.py:14343) and, in the defaultbusy_input_mode == "interrupt", puts that tuple on_interrupt_queue(cli.py:14395). The image path always goes there — theagent.redirect()fast path above it is gated onif not images and text.chat()'s requeue branch then ran:which raises
TypeError: sequence item 0: expected str instance, tuple found.pending_messageis itself bound from the queue value —result.get("interrupt_message") or interrupt_msg(cli.py:13298) andpending_message = interrupt_msg(cli.py:13312), andinterrupt_msgcomes straight off_interrupt_queue.turn_finalizer.py:625returnsagent._interrupt_messageverbatim, so the tuple survives the round trip intact. A single interrupted image message is enough to trigger the crash — a second queued message is not required.The failure is invisible and unrecoverable:
chat()'s whole body sits under a bareexcept Exception as e: print(f"Error: {e}")(cli.py:13492), so the user sees one line of error text and nothing else.get_nowait()before the join, so the message and its images are gone — not requeued, not re-displayed, not recallable.The fix merges the parts type-aware at the requeue site: join the text, concatenate the image lists, and emit a plain
strwhen no part carries images so the common text-only path is unchanged.process_loopalready unpacks the(text, images)form atcli.py:16132-16134, so the merged payload round-trips into the next turn as a real multimodal message.Sibling-site sweep
grep -n '_interrupt_queue\|_pending_input' cli.py— every producer and consumer of these two queues was enumerated. The type-blind join exists at exactly one site; the rest are already tuple-safe and are deliberately left untouched:cli.py:13475(chat()requeue)cli.py:14370(queue mode)_pending_input.put(payload)cli.py:14395(interrupt mode)_interrupt_queue.put(payload)cli.py:14423(agent idle)_pending_input.put(payload)cli.py:13117(clarify race)interrupt_msgin_pending_inputcli.py:10208-10211_drain_interrupt_queue_to_pending_input()cli.py:10255-10262goal-continuation peek(text, images)cli.py:6815alt submit pathstrcli.py:13488leftover/steerpreviewstr/steeris text-only by construction (the Enter handler falls back to queue mode when images are attached)No mirror of this join exists in
tui_gateway/,gateway/, orapps/desktop/—grep -rn "all_parts" --include='*.py' .returns this one production site.Related Issue
No filed issue — this supersedes open PR #5202 and implements the review left on it.
Type of Change
Changes Made
cli.py— replaced the type-blind"\n".join(all_parts)inHermesCLI.chat()'s interrupt-requeue branch with a type-aware merge: text joined with\nin arrival order, image lists concatenated in arrival order, result emitted as(text, images)only when at least one part carries images and as a plainstrotherwise.previewnow derives from the joined text (so it stays astr) and gains an[N images attached]note when attachments are present, including the text-empty case. The existingn > 1vs single-message print branch is unchanged.tests/cli/test_cli_interrupt_ack_race.py— added three tests on the liveHermesCLI.chat()path: a single image interrupt, a mixed text+image merge, and a text-only guard.How to Test
Reproduction (before this PR): attach an image with the agent running, press Enter (default
busy_input_mode = interrupt). The turn is interrupted, thenError: sequence item 0: expected str instance, tuple foundprints and your message and image never come back.Automated, on the live
chat()requeue branch:Fails-before / passes-after, verified in both directions by reverting only the
cli.pyhunk:main(prod hunk reverted)test_interrupt_with_attached_images_is_requeued_intact_pending_inputis empty; captured stdout shows the swallowedError: sequence item 0: expected str instance, tuple found("what is in this screenshot?", [Path(...)])test_interrupt_merges_text_and_image_parts_without_losing_attachmentsTypeError, all three parts losttest_text_only_interrupt_still_requeues_a_plain_stringFull results:
tests/cli/test_cli_interrupt_ack_race.py— 11 passed (8 pre-existing + 3 new).tests/cli/— 1232 passed, 1 failed. The single failure,test_resume_quiet_stderr.py::TestResumeQuietStderr::test_session_not_found_goes_to_stdout_in_full_mode, is a pre-existing baseline: the identicalpytest tests/cli/ -q -p no:randomly -n 4run on cleanorigin/main(ef2670113) fails the same test (1229 passed, 1 failed), and it passes in isolation on both. It is a parallel-ordering artifact, unrelated to this change and untouched by it.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/cli/ -qand all tests pass (one pre-existing baseline failure, reproduced on cleanorigin/main— see above)Documentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ARelated / Positioning
mattsegura, open) — same diagnosis, superseded here. See the header.nicoechaniz, closed unmerged 2026-04-28) — earlier attempt at the same defect.HH1162, open) — edits the same block but for a different concern (background-task input loss / old messages reappearing, across 5 files). Its requeue rewrite addsisinstance(extra, str)filters, which drops image payloads instead of preserving them — it converts this crash into a silent attachment loss rather than fixing it. The two changes are not substitutes.