fix(gateway): strip MEDIA tags before chained /queue follow-up#19011
Open
Tranquil-Flow wants to merge 2 commits into
Open
fix(gateway): strip MEDIA tags before chained /queue follow-up#19011Tranquil-Flow wants to merge 2 commits into
Tranquil-Flow wants to merge 2 commits into
Conversation
Collaborator
Collaborator
Contributor
Author
|
Hey @alt-glitch, just flagging that it seems your agent is double commenting here. Might be worth checking how the agent processes new PRs :) |
19 tasks
Multiple ``/queue`` items chained in one session via the FIFO queue lost attachments on every item except the last. The queue-drain path in ``_run_agent`` called ``adapter.send(first_response)`` with the raw, ``MEDIA:/path``-tagged response — so the per-item file markers leaked through as plain text, the file existed on disk, but the upload never happened. The normal ``_process_message_background`` path runs ``adapter.extract_media`` before ``adapter.send``; the queue path missed that gate. Extract the queue-drain delivery into ``_send_queued_followup_first_response`` and run the same ``extract_media`` → ``send(cleaned_text)`` → ``send_document`` / ``send_voice`` per file pipeline as the normal path. ``is_voice``-flagged files route through ``send_voice`` when the adapter exposes one, falling back to ``send_document`` so unsupported adapters still deliver instead of crashing. Per-file upload errors are logged but don't block the remaining files — losing one attachment beats losing all. Tests cover: extract_media-before-send ordering, multi-file dispatch through ``send_document``, voice-flagged routing through ``send_voice``, graceful continuation when one upload raises, and the no-MEDIA path preserving pre-fix behaviour exactly (one ``send`` call, no document uploads). Fixes NousResearch#18539
68e7c6a to
a77b571
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Multiple
/queueitems chained in one session via the FIFO queue lost attachments on every item except the last. The queue-drain path in_run_agentcalledadapter.send(first_response)with the raw,MEDIA:/path-tagged response — so per-item file markers leaked through as plain text, the file existed on disk, but the upload never happened. The normal_process_message_backgroundpath runsadapter.extract_mediabeforeadapter.send; the queue path missed that gate.Extract the queue-drain delivery into
_send_queued_followup_first_responseand run the sameextract_media→send(cleaned_text)→send_document/send_voiceper file pipeline as the normal path.is_voice-flagged files route throughsend_voicewhen the adapter exposes one, falling back tosend_documentso unsupported adapters still deliver instead of crashing. Per-file upload errors are logged but don't block remaining files — losing one attachment beats losing all.Related Issue
Fixes #18539
Type of Change
Changes Made
gateway/run.py— replace the inline raw-send block in_run_agent's queue-drain branch with a call to a new helper.gateway/run.py— new_send_queued_followup_first_response()method that mirrors the normal path's extract → send → upload-files pipeline.tests/gateway/test_queue_followup_media.py— new file, 5 tests: extract-before-send ordering, multi-document dispatch, voice routing, per-file failure containment, no-MEDIA preserves pre-fix behaviour.How to Test
/queuetwice with messages that produce MEDIA attachments (e.g. ask the agent for a chart and a PDF).MEDIA:/pathtext.pytest tests/gateway/test_queue_consumption.py tests/gateway/test_queue_followup_media.py -q→ 16/16 pass.Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & 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/AScreenshots / Logs