Skip to content

fix: handle non-image link previews - #42403

Open
bananawalnut wants to merge 1 commit into
NousResearch:mainfrom
bananawalnut:fix/open-graph-card-non-image-links-20260608150402
Open

fix: handle non-image link previews#42403
bananawalnut wants to merge 1 commit into
NousResearch:mainfrom
bananawalnut:fix/open-graph-card-non-image-links-20260608150402

Conversation

@bananawalnut

@bananawalnut bananawalnut commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Reject non-image bytes before native image attachment
  • Parse Open Graph/Twitter metadata when a URL resolves to HTML instead of image bytes
  • Route OpenAI Codex image attachments through text vision by default to avoid invalid image-data 400s

Test Plan

  • /Users/bananawalnut/.hermes/hermes-agent/venv/bin/python -m pytest tests/agent/test_image_routing.py tests/tools/test_vision_open_graph_card.py -q

@liuhao1024

liuhao1024 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Verification review — clean, no issues found.

What's good:

  • _sniff_mime_from_bytes replacing _guess_mime in _file_to_data_url is the right fix — extension-based trust is a classic source of data corruption when URL preview HTML gets saved as .png
  • The Open Graph parser uses html.parser.HTMLParser (stdlib, no new deps) and limits input to 1MB — reasonable for both memory and performance
  • _open_graph_card_from_file has a sensible HTML detection heuristic (<html, <!doctype html, <head>) that won't false-positive on binary blobs
  • The Codex provider routing to text-vision mode is documented and tested — a pragmatic workaround for a backend limitation
  • Test coverage: non-image bytes with image suffix, OG card extraction with relative URL resolution, and the async vision paths all covered

The _format_open_graph_card output is clean and informative — the agent gets structured metadata instead of a silent failure.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/vision Vision analysis and image generation P3 Low — cosmetic, nice to have labels Jun 8, 2026
teknium1
teknium1 reviewed Jul 14, 2026
View reviewed changes

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the focused regression coverage. The local non-image premise remains valid on current main: agent/image_routing.py:651-669 still falls back to an image MIME when magic-byte sniffing returns no result, so HTML named preview.png can be attached as image/png.

Problems

  • The Open Graph implementation targets the old post-download path. Current tools/vision_tools.py:970 and :1141 use tools.image_source.resolve_image_source, which rejects non-image bytes at tools/image_source.py:330-337 before those branches run.
  • The added async branches call synchronous _validate_image_url() (tools/vision_tools.py:207-213); the URL-safety module documents that DNS work as blocking and provides an async alternative at tools/url_safety.py:472-474.
  • The Codex routing hunk needs relocation for current main: agent/image_routing.py:445-447 returns native before a post-capability exception could run.

Suggested changes

  • Rework the HTML-card behavior through the unified resolver while preserving its URL-policy and sandbox-confinement checks, with tests for both vision entry points.
  • Keep recognized uncommon-image transcoding and reject only unrecognized bytes.
  • Put and test the Codex exception before the capability early return.

Automated hermes-sweeper review.

Comment thread tools/vision_tools.py
image_size_bytes = temp_image_path.stat().st_size
detected_mime_type = _detect_image_mime_type(temp_image_path)
if not detected_mime_type:
if _validate_image_url(image_url):

@teknium1 teknium1 Jul 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_validate_image_url() performs synchronous URL-safety/DNS work, but this branch runs inside an async function. Please use the existing async validation path (or carry forward the earlier validated resolver state) so this fallback cannot block the event loop.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 14, 2026
@withzombies withzombies mentioned this pull request Aug 9, 2026
Open
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@teknium1 teknium1 teknium1 left review comments

Assignees

No one assigned

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/vision Vision analysis and image generation type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bananawalnut @liuhao1024 @teknium1 @alt-glitch