Skip to content

fix(gateway): stop MEDIA tag regex from absorbing following tag or text (#68773) - #68781

Closed
Enough1122 wants to merge 1 commit into
NousResearch:mainfrom
Enough1122:fix/68773-media-tag-separator
Closed

fix(gateway): stop MEDIA tag regex from absorbing following tag or text (#68773)#68781
Enough1122 wants to merge 1 commit into
NousResearch:mainfrom
Enough1122:fix/68773-media-tag-separator

Conversation

@Enough1122

Copy link
Copy Markdown
Contributor

Fixes #68773.

Two MEDIA:<path> tags emitted back-to-back without a separator (MEDIA:/a.pngMEDIA:/b.png) merged into a single invalid path and were silently dropped. The same happened for extension-less tags (MEDIA:/CaddyfileMEDIA:/Dockerfile).

Root cause

Both regexes in gateway/platforms/base.py used greedy quantifiers in their path class:

  • MEDIA_TAG_CLEANUP_RE\S+(?:[^\S\n]+\S+)*?\.ext could capture the next MEDIA: keyword as part of the path.
  • MEDIA_EXTENSIONLESS_TAG_RE[^\s\n"']+` with no boundary lookahead absorbed the next tag's content.

The merged path (/a.pngMEDIA:/b.png) failed validate_media_delivery_path and was dropped. On Ollama-hosted GLM models, the leftover MEDIA: text in the unstripped source then tripped _should_treat_stop_as_truncated and injected a spurious "[System: Your previous response was truncated...]" continuation prompt.

Fix

Make both regexes non-greedy and add MEDIA: to the trailing-lookahead boundary set:

  • MEDIA_TAG_CLEANUP_RE: \S+(?:[^\S\n]+\S+)*?\.ext\S+?(?:[^\S\n]+\S+?)*?\.ext, plus quoted branches also become non-greedy. Lookahead adds |MEDIA: to its boundary set.
  • MEDIA_EXTENSIONLESS_TAG_RE: [^\s\n"']+[^\s\n`"']+?` followed by `(?=[`"'\s,;:)\]}]|MEDIA:|$)`.

Tests

tests/gateway/test_media_tag_separator.py — 7 new tests:

  • test_known_extension_regex_splits_glued_tags — primary regex matches each tag independently.
  • test_strip_media_directives_handles_glued_known_extension_tags — both real .png files delivered.
  • test_extensionless_regex_does_not_absorb_next_media_keyword — fallback regex stops at the next MEDIA:.
  • test_extensionless_regex_does_not_absorb_following_text — fallback regex stops at the next MEDIA: even when surrounded by text.
  • test_extensionless_regex_still_matches_normal_cases — well-formed paths still match.
  • test_strip_media_directives_handles_glued_extensionless_tags — both real files delivered.
  • test_strip_media_directives_does_not_drop_known_ext_tag_followed_by_text — known-extension tag glued to text leaves the text visible (no silent drop).

All 7 pass. Pre-existing failures in test_platform_base.py are unchanged (verified by stashing my diff and running the same tests against main).

— written by Hermes Agent on behalf of @Enough1122

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/gateway Gateway runner, session dispatch, delivery provider/ollama Ollama / local models needs-decision Awaiting maintainer decision before any implementation labels Jul 21, 2026
@PRATHAMESH75

Copy link
Copy Markdown
Contributor

Reviewed against #68773 — this correctly fixes the root cause (Bug 1), and I verified it locally.

Correctness

  • Both regexes switch the path token to non-greedy (+?) and add MEDIA: to the trailing lookahead. For MEDIA:/a.pngMEDIA:/b.png the primary regex now stops the first match at the MEDIA: boundary and matches the second independently, instead of the old greedy \S+ swallowing everything into one invalid /a.pngMEDIA:/b.png path. That's exactly the real-world case from the issue's log evidence (fr_main.png running into the next fr_add.png tag).
  • The non-greedy .(?:ext) is still guarded by the boundary lookahead, so legitimately dotted paths (/tmp/a.png.jpg, /tmp/a.b.png) don't stop prematurely — the lookahead only accepts a real separator / MEDIA: / $, so it expands to the correct extension. Good.
  • Not touching run_agent.py for Bug 2 (spurious truncation) is the right call: that misdetection is a downstream symptom of tags leaking into the visible text, so fixing extraction removes the leftover MEDIA: and the false-truncation trigger with it. No independent defect there.

Tests — ran the new tests/gateway/test_media_tag_separator.py (7 passed) plus the rest of the gateway media suite. No regressions from this change. (Heads-up unrelated to your PR: test_background_command.py::test_media_files_routed_by_type fails on macOS on plain upstream/main too — it's a /var vs /private/var tmpdir-symlink assertion artifact, not anything you touched.)

One minor, non-blocking note
The known-extension-glued-to-plain-text case (MEDIA:/file.pngSome text, no second tag) is intentionally left visible — your test_strip_media_directives_does_not_drop_known_ext_tag_followed_by_text pins cleaned == text, which is the safe choice since the boundary is genuinely ambiguous there. Worth being aware that in that specific (rarer) shape the file still isn't delivered and the response can still end on a non-natural char, so Bug 2's truncation heuristic could in principle still fire for it on Ollama/GLM. Not something I'd hold the PR for — the two-glued-tags case in the report is the common one and it's solidly fixed — just flagging the residual edge for the maintainer.

LGTM. Focused change, correct root cause, real regression coverage.

@Enough1122

Copy link
Copy Markdown
Contributor Author

@PRATHAMESH75 thanks for the careful review and the local verification — much appreciated.

Noted on the residual edge case for MEDIA:/file.pngSome text. The ambiguity there (path could plausibly end at any of .png, gSome, etc.) makes me agree that preserving the original text is the safer default — better visible-noise than silent-drop. If a maintainer wants to tighten it later, a follow-up PR with a heuristic like "stop at the first non-MEDIA: letter that doesn't extend a known extension" would be the path; happy to file that as a separate issue rather than scope-creep this one.

— written by Hermes Agent on behalf of @Enough1122

@Enough1122

Copy link
Copy Markdown
Contributor Author

cc @PRATHAMESH75 @alt-glitch — thanks again for the LGTM and the local verification.

Noted on the residual edge case for MEDIA:/file.pngSome text — preserving the original text is the safer default given the genuine path-boundary ambiguity there. If a maintainer wants a follow-up tightening heuristic, happy to file as a separate issue rather than scope-creep this PR.

Branch fix/68773-media-tag-separator unchanged. Ready for maintainer merge whenever.

— written by Hermes Agent on behalf of @Enough1122

…xt (NousResearch#68773)

Two MEDIA:<path> tags emitted back-to-back without a separator
(MEDIA:/a.pngMEDIA:/b.png) used to be merged into a single invalid
path by both:

1. MEDIA_TAG_CLEANUP_RE — the path class used greedy \S+(?:[^\S\n]+\S+)*?
   which could capture the next MEDIA: keyword as part of the path.
2. MEDIA_EXTENSIONLESS_TAG_RE — the path class used greedy
   [^\s\n`"']+ with no boundary lookahead, so the same merge happened
   for extension-less tags.

The merged path (/a.pngMEDIA:/b.png) failed
validate_media_delivery_path and was silently dropped. On Ollama-hosted
GLM models, the leftover MEDIA: text in the unstripped source then
tripped _should_treat_stop_as_truncated and injected a spurious
"[System: Your previous response was truncated...]" continuation prompt.

This change makes both regexes non-greedy and adds MEDIA: to the
trailing-lookahead boundary set, so each tag matches independently.

Tests in tests/gateway/test_media_tag_separator.py cover:
- known-extension regex splits glued tags
- known-extension strip delivers both files
- extensionless regex splits glued tags
- extensionless regex stops at next MEDIA:
- extensionless strip delivers both files
- extensionless regex still matches well-formed paths
- known-ext tag followed by text stays visible (no silent drop)
@Enough1122
Enough1122 force-pushed the fix/68773-media-tag-separator branch from b4e703d to ea76d75 Compare July 22, 2026 10:21
@Enough1122

Copy link
Copy Markdown
Contributor Author

cc @PRATHAMESH75 — rebased onto current upstream main. The fix is unchanged in content (2 files, +133/-4).

Thanks again for the LGTM. The branch was rebuilt from upstream main because the previous fork base had fallen 2955 commits behind — fixed today. The single-commit fix on gateway/platforms/base.py (regex +? non-greedy + MEDIA: boundary lookahead) plus the new tests/gateway/test_media_tag_separator.py (7 cases) are identical to what you reviewed.

tests/gateway/test_media_tag_separator.py 7 passed. git diff --check clean. Ready for maintainer merge.

— written by Hermes Agent on behalf of @Enough1122

@Enough1122

Copy link
Copy Markdown
Contributor Author

Closing this stale PR for now: it has not reached a mergeable/reviewed state and is unlikely to be merged in its current form. Reopen or submit a fresh PR if the issue remains relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery needs-decision Awaiting maintainer decision before any implementation P2 Medium — degraded but workaround exists provider/ollama Ollama / local models type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Agent fails to append separators after MEDIA tags, URLs, and file paths, causing cascading delivery and truncation failures

3 participants