← Back to PRs

#18811: fix(media): require file extension for ambiguous MEDIA: path detection

by aldoeliacim open 2026-02-17 03:04 View on GitHub →
size: XS trusted-contributor
## Summary Fixes #18780 — `MEDIA:` substrings in tool result text (e.g. from `web_fetch` of release notes) trigger false-positive file path detection, causing ENOENT errors on Telegram and other channels. ## Root Cause `isLikelyLocalPath()` in `src/media/parse.ts` has a catch-all condition that matches **any** string containing a `/` character: ```ts !SCHEME_RE.test(candidate) && (candidate.includes('/') || candidate.includes('\\')) ``` When a line starts with `MEDIA:` (e.g. `MEDIA:-prefixed paths (lenient whitespace) when loading...`), the text after the token contains `/` characters in prose, so `isLikelyLocalPath` returns `true`, and the handler tries to `fs.open()` the prose as a file path. ## Fix Add `HAS_FILE_EXT.test(candidate)` to the catch-all condition. This requires a file extension (`.ext`) for ambiguous relative paths like `subdir/file.png`. All explicit path prefixes (`/`, `./`, `../`, `~`, Windows drives, UNC) are checked by earlier conditions and remain unaffected. ## Tests - Added regression test for the exact scenario from #18780 - All existing media/parse tests pass (15/15) - All outbound/delivery tests pass (564/564) - All tools media tests pass (30/30) <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes false-positive `MEDIA:` path detection by adding a file extension requirement (`HAS_FILE_EXT.test()`) to the catch-all condition in `isLikelyLocalPath()`. This prevents prose text containing `/` characters from being treated as file paths when it appears after a `MEDIA:` token. - The code fix in `src/media/parse.ts` is correct and well-scoped — it only tightens the catch-all condition for ambiguous relative paths (those without explicit prefixes like `/`, `./`, `../`, `~`, drive letters, or UNC). All explicitly-prefixed paths are unaffected. - The regression test in `src/media/parse.test.ts` has a gap: the test string contains no `/` or `\` characters, so the catch-all condition would never have fired for this input even before the fix. The test passes on both old and new code and doesn't validate the `HAS_FILE_EXT` guard. The test input should include `/` in the prose to properly exercise the changed code path. <h3>Confidence Score: 4/5</h3> - The code fix is correct and safe to merge, though the regression test should be strengthened to actually exercise the fix. - The production code change is minimal, well-commented, and correctly addresses the bug. It reuses an existing regex (`HAS_FILE_EXT`) that is already battle-tested in the same file. All explicit path prefix handling is unaffected. The only concern is the regression test doesn't actually validate the fix — the test string lacks `/` characters needed to trigger the catch-all condition. This means the fix could regress without the test catching it. Deducting one point for this test gap. - `src/media/parse.test.ts` — the regression test should include `/` characters in the prose to properly exercise the `HAS_FILE_EXT` guard on the catch-all condition. <sub>Last reviewed commit: 346d480</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs