Conversation
b327673 to
9655de3
Compare
9655de3 to
0d0a378
Compare
0d0a378 to
cff4d66
Compare
8a85668 to
8bfe8dc
Compare
8bfe8dc to
9030100
Compare
9030100 to
727c310
Compare
727c310 to
4a4cf20
Compare
4a4cf20 to
570aa36
Compare
ea665aa to
e69b5e3
Compare
e69b5e3 to
d9803ab
Compare
d9803ab to
4c800dd
Compare
4c800dd to
b53f79d
Compare
b53f79d to
1165cea
Compare
1165cea to
5d152ef
Compare
5d152ef to
0a0a27a
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for narrowing this to the queued first-response resend path. The defect is present on current main: gateway/run.py:19724-19728 directly sends first_response, while native attachment routing lives in _deliver_media_from_response() at gateway/run.py:13097-13216.
Problems
- The added broad cleanup at
gateway/run.py:2188strips every remainingMEDIA:token afterextract_media(). That conflicts with the base contract: protected code/inline-code spans and unsupported or unvalidated tags are deliberately preserved (gateway/platforms/base.py:3623-3675,1460-1481). - The new tests call the helper directly, rather than taking the queued resend branch at
gateway/run.py:19718-19730; they do not verify the actual integration point.
Suggested changes
- Drop the broad regex and retain the base extractor's cleaned text semantics.
- Add a queued-follow-up integration test, including attachment routing and a protected
MEDIA:example.
Automated hermes-sweeper review.
|
Thanks for the heads-up on #18546 / #19011 — same gap on the queued first-response resend path. This PR keeps that path, but routes delivery through the existing text + media helper so attachments still get the normal allowlist / type-aware routing instead of a one-off extract-and-send loop. Also dropped the broad leftover Rebased on current main. Happy to close/consolidate if maintainers want a single landing PR — otherwise this should be ready for another look. |
Current-head correction: #25119 predates the other open queued-media PRs. It shares the queued first-response delivery outcome but uses a distinct helper shape, so this is a competing implementation rather than a PR-to-issue duplicate. Related: #18546, #46223, and #51805. |
|
Opened a small incremental follow-up directly on top of this branch: pebble-tech#56. It covers the one remaining gap in the queued-follow-up flush: the "already streamed" branch (where streaming already delivered the first response's text) never ran the Also closing #77732 — a duplicate that a triage pass flagged as part of this same family — in favor of consolidating that one missing piece here rather than competing with your work. |
Ensure queued follow-up resends keep MEDIA-backed attachments by replaying the first response through the gateway's text-plus-media delivery flow instead of a plain adapter text send.
Drop the broad MEDIA: regex after extract_media so code/inline examples survive, and cover the real queued first-response resend path in tests.
What does this PR do?
When a second inbound message queues behind an in-progress gateway turn, the queued resend path previously replayed only the text body. MEDIA-backed attachments from
send_assetcould be dropped silently. This routes queued first-response resends through a text-plus-media delivery helper instead of a plain adapter text send.Related Issue
Fixes #
Type of Change
Changes Made
gateway/run.py— route queued first-response resends through_deliver_queued_first_response; preserve MEDIA attachments the queued path can replay nativelytests/gateway/test_tts_media_routing.py— regression test for queued follow-up media deliveryHow to Test
uv run pytest -o addopts= tests/gateway/test_tts_media_routing.py -k queued -qMEDIA:attachments, then queue a second inbound before delivery completes — confirm images are delivered on the queued resend pathChecklist
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/AFor New Skills
N/A — delete this section otherwise.
Screenshots / Logs
N/A