Skip to content

fix(image): detect Windows local image refs - #34374

Open
zapabob wants to merge 1 commit into
NousResearch:mainfrom
zapabob:codex/windows-image-refs
Open

fix(image): detect Windows local image refs#34374
zapabob wants to merge 1 commit into
NousResearch:mainfrom
zapabob:codex/windows-image-refs

Conversation

@zapabob

@zapabob zapabob commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Detect native Windows drive-letter image paths in extract_image_refs().
  • Keep the existing URL-fragment guard so local path matching does not attach paths from https://... URLs.
  • Expand ~/... using HOME when present so tests and Windows shell environments behave predictably.

Why

The v0.15.1 kanban worker image-ref tests exercise local image attachments from task bodies. On Windows, absolute paths such as C:\Users\...\screenshot.png were not matched because the local path regex only handled POSIX /... and ~/... shapes. That caused local image paths to be dropped while URL images still attached.

I searched the upstream issue tracker for Windows extract_image_refs, kanban image Windows, and image path Windows and did not find an existing matching issue.

Verification

  • PYTHONUTF8=1 PYTHONIOENCODING=utf-8 .\.venv\Scripts\python.exe scripts\run_tests_parallel.py tests\agent\test_image_routing.py tests\hermes_cli\test_kanban_worker_image_extraction.py -- --tb=short --timeout-method=thread → 86 passed
  • .\.venv\Scripts\python.exe -m ruff check agent\image_routing.py tests\agent\test_image_routing.py tests\hermes_cli\test_kanban_worker_image_extraction.py → passed

@zapabob
zapabob force-pushed the codex/windows-image-refs branch from 466d1c2 to 8a109a1 Compare May 29, 2026 04:51
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint labels May 29, 2026

@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 Windows parity fix. The premise is confirmed on current main: agent/image_routing.py:68-70 still recognizes only POSIX / and ~/ paths, while the kanban worker calls extract_image_refs() on task bodies at cli.py:16065-16090. The gateway already uses the intended drive-letter grammar at gateway/platforms/base.py:3748-3754.

Problems

  • This PR changes only agent/image_routing.py; it adds no regression coverage. The existing extractor tests at tests/agent/test_image_routing.py:523-639 cover POSIX paths and URL guarding but not either Windows drive-letter separator form.

Suggested changes

  • Add focused tests for C:\\...\\shot.png and C:/.../shot.png, along with C:relative.png non-matching and the existing URL-fragment guard. Matcher-level cases can run cross-platform; add a native-Windows filesystem case if needed for path validation.

Automated hermes-sweeper review.

Comment thread agent/image_routing.py
# Absolute / home-relative local image path. Matches the same shape gateway's
# extract_local_files() uses: anchors to ``~/`` or ``/``, ignores matches inside
# URLs (the ``(?<![/:\w.])`` lookbehind), and case-insensitive on the extension.
# extract_local_files() uses, plus native Windows drive paths. The lookbehind

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.

Please add regression coverage for both C:\\...\\image.png and C:/.../image.png, plus a relative-drive and URL-fragment negative case. Current extractor tests cover only POSIX-shaped paths.

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

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants