fix(gateway): preserve queued follow-up media continuity - #71031
StellarisW wants to merge 7 commits into
Conversation
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.
MEDIA:/tmp paths are filtered by delivery safe roots; mirror other tests by placing the fixture under an allowed cache directory.
Drop the broad MEDIA: regex after extract_media so code/inline examples survive, and cover the real queued first-response resend path in tests.
|
Thanks for consolidating the queued-media repair. The premise remains verified on current main: The PR's text/media split and reuse of the queued turn's routing metadata fit that existing delivery contract. Its fallback and confirmed-stream coverage addresses both current branches. The branch is conflicting after the July 29 gateway refactor, but the required salvage is a narrow, conflict-aware reapplication around This is an automated hermes-sweeper review. |
SummaryTwo PRs address the queued-delivery bypass in Issue #60845. #25119 fixes the fallback queued resend path by routing text and explicit MEDIA attachments through the existing delivery helper; #71031 consolidates that path and additionally fixes explicit-media delivery when the first response was already streamed before queued recursion. Related pull requests
Duplicates#25119 and the fallback portion of #71031 implement the same explicit-MEDIA queued-resend repair; #71031 is the broader consolidation because it also covers media after confirmed streamed text. Suggested consolidationkeep open with a salvage path for #71031: rebase or reapply its verified text/media split and streamed-branch handling around the current gateway implementation, retaining the fallback and streamed regression tests. After that salvage is complete, close #25119 as duplicate/superseded by #71031; this explicitly addresses the keep_open review on #25119 because #71031 contains its salvageable fix while covering the additional streamed path. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I60845(["issue #60845 (open)"])
subgraph Dup25119 ["PRs duplicating each other"]
P25119["PR #25119 (open)"]
P71031["PR #71031 (open)"]
end
P71031 -->|best fix| I60845
class I60845 open
class P25119 open
class P71031 open
class P71031 best
class P71031 target
click I60845 "https://github.com/NousResearch/hermes-agent/issues/60845"
click P25119 "https://github.com/NousResearch/hermes-agent/pull/25119"
click P71031 "https://github.com/NousResearch/hermes-agent/pull/71031"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 2 pull requests and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 40 kB of PR diffs, 8 kB of issue/PR text, 6 kB of discussion (10 comments), 4 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
|
Merged via #82162 with all seven of your commits and authorship preserved (rebase-merge; d220f1f … a52dd17 on main), plus one follow-up commit (0b17b69) adding the failed-result guard: failed first turns deliver their normalized failure text but skip attachment upload, mirroring the completed-turn path. This was the strongest of the five PRs attacking this bug — the only one covering BOTH branches (fallback text leak + streamed attachment drop) while preserving the explicit-only delivery policy, thread routing, and protected MEDIA examples, with exactly-once assertions. The consolidation of @vKongv's #25119 commits with attribution was handled well. Verified against the full tests/gateway/ suite (5101 pass). Closing this original since the salvage PR carried it in — thanks for the persistent, careful work here. |
What does this PR do?
Queued follow-up delivery had a separate direct-send path from normal final-response delivery. That path could expose a literal
MEDIA:directive instead of uploading the attachment, and it skipped explicit media entirely when the first response text had already been streamed.This PR keeps queued text and attachment delivery continuous across both fallback and streamed cases:
This is an attribution-preserving consolidation of #25119. Its three commits remain in the branch with Kong Ka Weng (
vKongv) as author; the additional commits add current-maincompatibility and cover the streamed sibling path.Related Issue
Fixes #60845
Related: #18539, #55806, #19011, #25119, #46223, #51805
Type of Change
Changes Made
gateway/run.pytests/gateway/test_run_progress_topics.pytests/gateway/test_tts_media_routing.pyHow to Test
pytest tests/gateway/test_run_progress_topics.py tests/gateway/test_post_stream_media_delivery.py tests/gateway/test_tts_media_routing.py -p no:cacheprovider -o 'addopts=' -q63 passed.14 passed, 1 skipped.ruff check . python scripts/check-windows-footguns.py --allChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/A: no user-facing configuration or API changecli-config.yaml.exampleif I added/changed config keys — or N/A: no config changeCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/A: no architecture/workflow changecheck-windows-footguns.py --allpassesScreenshots / Logs
Not applicable. The regression is covered by gateway integration tests that assert observable text, attachment, recursion, and routing behavior.