Conversation
SimpleX has no album concept -- sending 2+ images from the iOS app arrives as separate chat items dispatched immediately, so image 2 lands mid-agent-run and triggers an "Interrupting current task" ack instead of being treated as part of the same message. Widen the existing text quiet-period batcher to also cover PHOTO/DOCUMENT/VOICE events, and fix the merge path to recompute message_type (VOICE > PHOTO > DOCUMENT > TEXT) instead of leaving it stuck at the first event's type. Media delay defaults to the existing 0.8s text delay and can be widened independently via platforms.simplex.extra.media_batch_delay for slower XFTP transfers, without adding a new env var. A quiet period alone is not enough in practice: the chat items of a multi-attachment send arrive together, but their XFTP downloads complete serially and regularly land further apart than any sane quiet period. Since the adapter already knows which transfers are still pending, the flush now holds the batch open while a sibling transfer for the same chat is downloading (bounded by a 60s cap so a stalled transfer cannot hold the chat's messages hostage), instead of guessing at a longer delay.
tonydwb
left a comment
There was a problem hiding this comment.
Code Review Summary
Verdict: Approved with notes
PR #66141 fix(simplex): batch multi-attachment sends into one event instead of interrupting the run
Assessment
- Correctness: SimpleX multi-attachment sends were arriving as separate chat items (interrupting the run per attachment). The fix batches them by holding the batch open while sibling file transfers are still on the wire, with MEDIA_BATCH_MAX_HOLD (60s) as an upper bound to prevent stalled transfers from holding messages hostage. _message_type_for_media() classifies batched media by priority (audio > image > document > text).
- Scope: ~560 lines, focused on the batching logic. Consistent with the existing text batching pattern.
- Testing: Docstring documents the behavior clearly; logic is sound.
- Note: The MEDIA_BATCH_MAX_HOLD cutoff at 60s is a reasonable upper bound. The except clause that flushes on any error in _build_batch_hold_tasks is appropriate.
Reviewed by Hermes Agent
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the focused SimpleX investigation. The current-main batching premise is confirmed at plugins/platforms/simplex/adapter.py:661-667, but this needs a few correctness changes before salvage.
Problems
- Incomplete image/document transfers still bypass the new hold path: current main's audio-only guard remains at
plugins/platforms/simplex/adapter.py:580, and this PR only records the batch key inside that guard. The full XFTP image path therefore still depends on #55185. - Reclassifying a mixed voice/image batch as
VOICEroutes every attachment to STT:gateway/run.py:10830-10836appends every path whenevent.message_type == MessageType.VOICE, irrespective of its MIME type. - The retained key at
plugins/platforms/simplex/adapter.py:673-675is chat-scoped, while group sender identity comes fromgroupMemberat:526-530; rapid attachments from distinct members can merge under the first sender.
Suggested changes
- Compose with the all-file deferral fix and add a pending-image completion regression.
- Add a gateway-path mixed voice/image test and make audio routing attachment-MIME-aware before using VOICE as an aggregate type.
- Use a sender/session-aware group batch key and test two concurrent group members.
Automated hermes-sweeper review.
| file_id, | ||
| ) | ||
| self._pending_file_transfers[file_id] = chat_item | ||
| self._pending_transfer_batch_keys[file_id] = ( |
There was a problem hiding this comment.
This new batch-key tracking only runs inside the existing _is_audio_ext(ext) pending-file branch. Incomplete XFTP images/documents still never enter the pending-transfer map or this hold path; please compose this with the all-file deferral change and cover an image completion event.
| existing.media_types.extend(event.media_types) | ||
| existing.message_type = _message_type_for_media(existing.media_types) | ||
|
|
||
| prior_task = self._pending_text_batch_tasks.get(key) |
There was a problem hiding this comment.
A mixed image+voice batch becomes VOICE here, but gateway/run.py:10830-10836 treats every path in a VOICE event as audio and sends it to STT. Please either make downstream audio selection per-attachment or avoid a whole-event VOICE type for mixed batches, with an end-to-end regression test.
SummaryFour open PRs touch the SimpleX integration, but their diffs address distinct causes: invalid contact-discovery documentation (#29520), spoofable allowlist matching (#41246), unresolved daemon-relative attachment paths (#59974), and separately dispatched multi-attachment events (#66141). They are related through the SimpleX integration, not competing implementations of one fix. Related pull requests
Suggested consolidationKeep #29520 open with a salvage path for the localized documentation, current-main reconciliation, and discovery-cache prerequisite. Author action on #41246: rebase onto main or split out the authorization fix with current adapter wiring and end-to-end coverage; keep #59974 open with its config-based path-resolution logic for current-head validation, and keep #66141 open until the contributor-identified transfer, mixed-media routing, and group-sender isolation blockers are addressed. None of these PRs should be closed as duplicates because their inspected diffs address separate causes. Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 45 kB of PR diffs, 18 kB of issue/PR text, 5 kB of discussion (5 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
What does this PR do?
Stops a multi-attachment send from interrupting the agent's in-flight run. SimpleX has no album concept: sending 2+ images as one message from a phone client delivers each image as its own chat item with its own XFTP transfer. The adapter dispatched every non-text item immediately, so image 1 started an agent run and image 2 arrived seconds later into the busy session, with the default
busy_input_mode: interrupt, the user gets "⚡ Interrupting current task. I'll respond to your message shortly." instead of one reply that saw both images.The adapter already batches rapid-fire text through a quiet-period batcher (mirroring Telegram), and the batcher's merge path already merges
media_urls/media_types; that branch was just unreachable, because the dispatch gate only routedTEXTevents into it.This PR:
PHOTO/DOCUMENT/VOICEevents flow through the same quiet-period batcher (a voice-note burst is the same shape as an image pair).message_typeon every merge (VOICE > PHOTO > DOCUMENT > TEXT, the same priority used to classify a single message); previously a batch stayed stuck at the first event's type, so text-then-image would dispatch asTEXTand the gateway's media handling would never see the attachment.platforms.simplex.extra.media_batch_delay(defaults to the existing text batch delay; no new env var, perAGENTS.md; behavioral settings live inconfig.yaml).Verified end-to-end on a live deployment (Fedora, Hermes v0.18.0, simplex-chat v6.5.5, iOS client).
Before (two images sent as one message split into two events and the second interrupted the run):
(the two completions landed ~1.4s apart, outside any reasonable quiet window, which is why the pending-transfer hold exists rather than a bigger delay).
After (one event carrying both images plus the caption, one agent reply, no interrupt):
Note: on current
main, inbound images reach the deferral path (and therefore the pending-transfer hold) only once the audio-only/freceiveguard is generalized; #55180 / PR #55185, same dependency #59974 has. This fix stands alone for voice-note bursts and captioned sends, and composes with #55185 for the full multi-image flow. Telegram has the same class of bug on file (#53198, #31541); this is the SimpleX sibling. Also adjacent but orthogonal: #63163 changes the batch key (per-sender authorship in groups); this PR changes what the batch accepts (they compose).Related Issue
None found: I searched open/closed issues and PRs for
simplex image batch,simplex multiple images,simplex interrupt,album,simplex batching; nearest matches are the Telegram analogs (#53198/#31541) and the complementary #55185. Root-cause analysis included above in lieu of a pre-filed issue; I am happy to split one out if preferred.Type of Change
Changes Made
plugins/platforms/simplex/adapter.py: routePHOTO/DOCUMENT/VOICEthrough_enqueue_text_event; extract_message_type_for_media()and recompute the merged event's type on every merge; track the batch key of each pending transfer and hold_flush_text_batchwhile siblings are downloading (MEDIA_BATCH_MAX_HOLD = 60scap); readextra.media_batch_delay(defaults to the text delay); generalize the flush log line; document the setting in a module-docstringconfig.yamlsection.tests/gateway/test_simplex_plugin.py: six new tests: two photos in one window become onePHOTOevent with bothmedia_urls; text-then-image upgrades the merged type; mixed media resolves byVOICE > PHOTO > DOCUMENTprecedence; a single image still dispatches after the window; the flush holds while a sibling transfer is pending (driven through the realrcvFileCompletepath) and flushes both once it lands; the hold gives up at the cap. Config goes throughPlatformConfig(extra={"media_batch_delay": ...}).website/docs/user-guide/messaging/simplex.md: documentplatforms.simplex.extra.media_batch_delaywith a config example.How to Test
Flushing message batch … (N chars, 2 media items)line, one agent reply that references both images.scripts/run_tests.sh tests/gateway/test_simplex_plugin.py(36 pass).Checklist
Code
fix(simplex): ...)Infographic