Skip to content

fix(agent): match Windows drive-letter paths in extract_image_refs - #61571

Open
lEWFkRAD wants to merge 1 commit into
NousResearch:mainfrom
lEWFkRAD:fix/image-refs-windows-paths
Open

fix(agent): match Windows drive-letter paths in extract_image_refs#61571
lEWFkRAD wants to merge 1 commit into
NousResearch:mainfrom
lEWFkRAD:fix/image-refs-windows-paths

Conversation

@lEWFkRAD

@lEWFkRAD lEWFkRAD commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a native-Windows product bug in agent/image_routing.py::extract_image_refs: the local-path regex (_LOCAL_IMAGE_PATH_RE) only anchored on ~/ or / with forward-slash separators, so Windows absolute paths (C:\Users\...\shot.png, C:/...) could never match. User-visible effect: kanban task-body image enrichment (cli.py) silently attached nothing on native Windows — pasting a screenshot path into a task body worked on POSIX and was a no-op on Windows. 12 tests fail on native Windows because of this (7 in tests/agent/test_image_routing.py::TestExtractImageRefs, 5 in tests/hermes_cli/test_kanban_worker_image_extraction.py).

The regex documents itself as matching "the same shape gateway's extract_local_files() uses" — but the gateway pattern was extended for Windows drive-letter paths in #34632 and this one was missed. This PR applies the exact same shape: (?:~/|/|[A-Za-z]:[/\]) anchor with [/\] segment separators.

Two adjacent fixes ride along:

  1. Returned paths are now normalized (os.path.normpath after expanduser) so the same file cited as C:\x\a.png and C:/x/a.png dedupes to one attachment, and tilde expansion on Windows doesn't leave a foreign / in the result. On POSIX this is lexical-identity for the shapes the regex emits — behavior unchanged.
  2. test_finds_home_relative_path monkeypatched only HOME, which ntpath.expanduser ignores (Python ≥3.8 uses USERPROFILE). It now sets both — same idiom as tests/tools/test_vision_tools.py.

Related Issue

Fixes #61568

Continues the native-Windows series: #57016, #57181 (fixes #57145), #57182 (fixes #57147), #57885 (fixes #57883). Windows-path parity with #34632.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • agent/image_routing.py: _LOCAL_IMAGE_PATH_RE gains the Bug: MEDIA directive silently fails on Windows #34632 drive-letter alternation and [/\] separators; expanded paths are normpath-normalized before the isfile check and dedupe; docstring/comments updated
  • tests/agent/test_image_routing.py:
    • test_finds_home_relative_path sets USERPROFILE alongside HOME
    • new test_mixed_separator_duplicates_collapse (native-separator + forward-slash spellings of one file → one attachment)
    • new TestLocalImagePathRegexWindowsShapes — pattern-level coverage that runs on every platform (backslash drive path, forward-slash drive path, lowercase drive, drive-root file, relative path and C:file.png non-matches, Unix shapes still match, URL path-portion still rejected via the lookbehind) — same approach as tests/gateway/test_run_tool_media_re.py

How to Test

  1. On native Windows, before this change: python -m pytest tests/agent/test_image_routing.py tests/hermes_cli/test_kanban_worker_image_extraction.py -q12 failed (assert [] == ['C:\...\real.jpg'])
  2. After this change, same command → 115 passed, 1 skipped
  3. On POSIX: the new pattern-level tests exercise the Windows shapes without needing files on disk; existing behavior is unchanged (anchor alternation only adds drive-letter roots; normpath is identity for the matched shapes)

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass (both affected files green natively on Windows; note below on pre-existing unrelated flakes)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: Windows 11 (native, Python 3.11)

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — docstring + regex comments updated in place
  • I've updated cli-config.yaml.example if I added/changed config keys — N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — that is the point of this PR; POSIX behavior unchanged
  • I've updated tool descriptions/schemas if I changed tool behavior — N/A

Screenshots / Logs

Before (main @ daedf4f, native Windows):

FAILED tests/agent/test_image_routing.py::TestExtractImageRefs::test_finds_absolute_path
FAILED tests/agent/test_image_routing.py::TestExtractImageRefs::test_finds_home_relative_path
FAILED tests/agent/test_image_routing.py::TestExtractImageRefs::test_dedupes_paths_and_urls
FAILED tests/agent/test_image_routing.py::TestExtractImageRefs::test_ignores_paths_in_fenced_code_block
FAILED tests/agent/test_image_routing.py::TestExtractImageRefs::test_ignores_paths_in_inline_code
FAILED tests/agent/test_image_routing.py::TestExtractImageRefs::test_mixed_paths_and_urls
FAILED tests/agent/test_image_routing.py::TestExtractImageRefs::test_case_insensitive_extension
FAILED tests/hermes_cli/test_kanban_worker_image_extraction.py::TestExtractFromTaskBody::test_local_path_in_body_round_trips
FAILED tests/hermes_cli/test_kanban_worker_image_extraction.py::TestExtractFromTaskBody::test_mixed_path_and_url_in_body
FAILED tests/hermes_cli/test_kanban_worker_image_extraction.py::TestBuildPartsFromTaskBody::test_local_path_becomes_native_image_part
FAILED tests/hermes_cli/test_kanban_worker_image_extraction.py::TestBuildPartsFromTaskBody::test_body_with_both_yields_two_image_parts
FAILED tests/hermes_cli/test_kanban_worker_image_extraction.py::TestBuildPartsFromTaskBody::test_code_block_example_is_not_attached
12 failed, ...

After:

115 passed, 1 skipped in 3.75s

Note: when test_image_routing.py runs together with the other vision-routing test files, TestDecideImageInputMode / TestLookupSupportsVisionOverride show order-dependent flaky failures on clean main as well (verified A/B, varying membership run to run) — pre-existing and unrelated to this change; flagged in #61568 and I'll file separately once isolated.

_LOCAL_IMAGE_PATH_RE only anchored on ~/ or / with forward-slash
separators, so Windows absolute paths (C:\Users\...\shot.png) never
matched and kanban task-body image enrichment silently attached nothing
on native Windows. The gateway's extract_local_files() pattern — which
this regex documents itself as mirroring — was already extended for
drive-letter paths in NousResearch#34632; this applies the same shape here.

Also normalize the expanded path (os.path.normpath) so mixed-separator
mentions of the same file dedupe to one attachment, and fix the tilde
test to set USERPROFILE alongside HOME (ntpath.expanduser ignores HOME).

Fixes NousResearch#61568

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@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 platform/windows Native Windows-specific behavior or breakage sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 9, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #34374 (earliest open fix PR for the same agent/image_routing.py::_LOCAL_IMAGE_PATH_RE Windows drive-letter gap, targeting the same code site with the same #34632-style regex mechanism). The normpath dedup and test-monkeypatch fix here are minor extras and don't change the core fix. Related: #61568 (the bug spec this addresses). Maintainer to pick the canonical PR.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused Windows parity fix. Current main still has the reported gap: agent/image_routing.py:68-70 only accepts ~/ and / roots and / path separators, while the established gateway implementation accepts [A-Za-z]:[/\\] and both separator styles at gateway/platforms/base.py:3725-3731. The PR's regex aligns this extractor with that existing behavior, and cli.py:16061-16080 confirms the affected kanban-worker image-enrichment path.

The existing member triage note identifies open PR #34374 as an earlier implementation of the same fix; choosing the canonical contribution is maintainer triage rather than a correctness concern with this patch.

Automated hermes-sweeper review.

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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists platform/windows Native Windows-specific behavior or breakage 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-platform-windows Sweeper risk: may break or behave differently on native Windows type/bug Something isn't working

Projects

None yet

3 participants