fix: save image to temp file and include path in prompt for Claude CLI#140
fix: save image to temp file and include path in prompt for Claude CLI#140haripatel07 wants to merge 3 commits intoRichardAtCT:mainfrom
Conversation
Image uploads were silently broken — process_image() built a text prompt saying 'I'm sharing a screenshot' but never passed the image data to Claude. The Claude CLI SDK only accepts text prompts, so the image was unreachable. Fix: download the image bytes to /tmp/claude_bot_files/image_<uuid>.<ext> and embed the file path in the prompt so Claude CLI can read the image directly from disk. All four prompt builders (_create_screenshot_prompt, _create_diagram_prompt, _create_ui_prompt, _create_generic_prompt) updated to accept and include the image_path parameter. Closes RichardAtCT#137
There was a problem hiding this comment.
Pull request overview
Fixes broken image uploads by saving Telegram photo bytes to a temp file and embedding that file path into the prompt sent to Claude CLI, allowing the CLI to access the actual image content from disk.
Changes:
- Save downloaded image bytes to
/tmp/claude_bot_files/image_<uuid>.<ext>. - Embed the saved file path into all image prompt builders (screenshot/diagram/UI/generic).
- Include the temp file path in
ProcessedImage.metadata.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/bot/features/image_handler.py
Outdated
| temp_dir = Path("/tmp/claude_bot_files") | ||
| temp_dir.mkdir(exist_ok=True) | ||
| image_path = temp_dir / f"image_{uuid.uuid4()}.{ext}" | ||
| image_path.write_bytes(bytes(image_bytes)) |
There was a problem hiding this comment.
The uploaded bytes are written to a fixed directory under /tmp without restricting permissions. By default this directory/file may be world-readable (depending on umask), which is risky for screenshots that can contain sensitive info. Consider creating the directory with a restrictive mode (e.g., 0o700) and writing the file with 0o600, or using tempfile APIs that create private files.
src/bot/features/image_handler.py
Outdated
| temp_dir = Path("/tmp/claude_bot_files") | ||
| temp_dir.mkdir(exist_ok=True) | ||
| image_path = temp_dir / f"image_{uuid.uuid4()}.{ext}" | ||
| image_path.write_bytes(bytes(image_bytes)) | ||
|
|
||
| # Create appropriate prompt | ||
| # Detect image type for prompt tailoring | ||
| image_type = self._detect_image_type(bytes(image_bytes)) | ||
|
|
||
| # Build prompt with actual file path so Claude CLI can see the image | ||
| if image_type == "screenshot": | ||
| prompt = self._create_screenshot_prompt(caption) | ||
| prompt = self._create_screenshot_prompt(caption, image_path) | ||
| elif image_type == "diagram": | ||
| prompt = self._create_diagram_prompt(caption) | ||
| prompt = self._create_diagram_prompt(caption, image_path) | ||
| elif image_type == "ui_mockup": | ||
| prompt = self._create_ui_prompt(caption) | ||
| prompt = self._create_ui_prompt(caption, image_path) | ||
| else: | ||
| prompt = self._create_generic_prompt(caption) | ||
| prompt = self._create_generic_prompt(caption, image_path) | ||
|
|
||
| # Convert to base64 for Claude (if supported in future) | ||
| base64_image = base64.b64encode(image_bytes).decode("utf-8") | ||
|
|
||
| return ProcessedImage( | ||
| prompt=prompt, | ||
| image_type=image_type, | ||
| base64_data=base64_image, | ||
| size=len(image_bytes), | ||
| metadata={ | ||
| "format": self._detect_format(image_bytes), | ||
| "format": fmt, | ||
| "has_caption": caption is not None, | ||
| "temp_path": str(image_path), | ||
| }, | ||
| ) |
There was a problem hiding this comment.
This introduces a new persistent temp file on disk, but nothing deletes it after Claude finishes. Over time this can fill /tmp and leak old user data. Suggest adding a cleanup path (e.g., have the caller delete ProcessedImage.metadata['temp_path'] in a finally block after run_command, or implement a TTL-based cleanup routine for /tmp/claude_bot_files).
src/bot/features/image_handler.py
Outdated
| # Detect format and save to temp file so Claude CLI can read it | ||
| fmt = self._detect_format(bytes(image_bytes)) | ||
| ext = fmt if fmt != "unknown" else "jpg" | ||
| temp_dir = Path("/tmp/claude_bot_files") | ||
| temp_dir.mkdir(exist_ok=True) | ||
| image_path = temp_dir / f"image_{uuid.uuid4()}.{ext}" | ||
| image_path.write_bytes(bytes(image_bytes)) |
There was a problem hiding this comment.
If _detect_format() returns "unknown", the code still writes the bytes to disk and forces a .jpg extension. That can produce misleading files and bypass the existing size/format checks in validate_image(). Consider validating (size + known format) before writing, and raising a clear error instead of defaulting to .jpg.
src/bot/features/image_handler.py
Outdated
| """Process uploaded image — save to temp file and build a path-based prompt.""" | ||
| import uuid | ||
|
|
||
| # Download image | ||
| # Download image bytes | ||
| file = await photo.get_file() | ||
| image_bytes = await file.download_as_bytearray() | ||
|
|
||
| # Detect image type | ||
| image_type = self._detect_image_type(image_bytes) | ||
| # Detect format and save to temp file so Claude CLI can read it | ||
| fmt = self._detect_format(bytes(image_bytes)) | ||
| ext = fmt if fmt != "unknown" else "jpg" | ||
| temp_dir = Path("/tmp/claude_bot_files") | ||
| temp_dir.mkdir(exist_ok=True) | ||
| image_path = temp_dir / f"image_{uuid.uuid4()}.{ext}" | ||
| image_path.write_bytes(bytes(image_bytes)) | ||
|
|
||
| # Create appropriate prompt | ||
| # Detect image type for prompt tailoring | ||
| image_type = self._detect_image_type(bytes(image_bytes)) |
There was a problem hiding this comment.
Minor maintainability: uuid is imported inside the method and the image bytes are repeatedly converted via bytes(image_bytes). Consider importing uuid at module scope (consistent with FileHandler) and assigning bytes_data = bytes(image_bytes) once to reuse for format/type detection and write_bytes.
|
PR Review Summary
What looks good
Issues / questions
Suggested tests (if needed)
Verdict — Friday, AI assistant to @RichardAtCT |
RichardAtCT
left a comment
There was a problem hiding this comment.
PR Review
Reviewed head: b00d01812c1ff0761b3f8d8b40117d027d6546cc
Summary
- Fixes silently broken image uploads by saving the image to
/tmp/claude_bot_files/and embedding the file path in the prompt, since the Claude CLI SDK only accepts text prompts - Correct diagnosis of the root cause; approach is pragmatic given the SDK constraint
What looks good
- Root cause analysis is solid and the fix is the right shape given Claude CLI's text-only prompt interface
exist_ok=Trueonmkdiravoids race conditions;/tmpalways exists so noparents=Trueneededfmtis reused for both the extension and the metadata, avoiding a redundant second_detect_format()call- 491 tests passing with no regressions
Issues / questions
-
[Blocker]
src/bot/features/image_handler.py— Does embedding a file path in the prompt actually cause Claude CLI to read the image? The PR asserts this works but provides no evidence. If Claude CLI doesn't auto-read paths mentioned in prompts (it likely doesn't unless explicitly invoked with a file argument), the fix resolves nothing — it just changes the error from "I don't see an image" to "I can't read that path." Needs a smoke-test screenshot or confirmation of which Claude CLI flag/behaviour makes this work before merging. -
[Important]
src/bot/features/image_handler.py— No temp file cleanup. Every image processed writes a file that is never deleted. On a busy bot this is an unbounded disk leak. Add atry/finallythat unlinks the path afterrun_command()completes, or usetempfile.NamedTemporaryFile(delete=True)scoped to the handler call. -
[Important]
src/bot/features/image_handler.py—import uuidis inside the function body. CLAUDE.md explicitly requires module-level imports. Move it to the top of the file next to the other stdlib imports. -
[Important]
src/bot/features/image_handler.py—ProcessedImage.base64_datais now dead code. The base64 is still computed and stored but nothing uses it (the comment# Convert to base64 for Claude (if supported in future)was removed). Either remove the field + computation to avoid confusion, or leave the comment explaining why it's retained. As-is it's misleading. -
[Nit]
src/bot/features/image_handler.py—_detect_image_type()is now called withbytes(image_bytes)after_detect_format()already converted it. The double conversion is harmless but consider assigningimage_bytes_bytes = bytes(image_bytes)once at the top and reusing it.
Suggested tests (if needed)
- Integration test: upload an image, assert the generated prompt string contains a valid path under
/tmp/claude_bot_files/, and assert that path exists on disk at the timerun_command()is called - Unit test: assert temp file is cleaned up after processing completes (once cleanup is added)
- Confirm (manually or in CI) that the Claude CLI actually reads the image when given a path in the prompt text — this is the core assumption the fix rests on
Verdict
import uuid is a quick style fix. More critically, the fundamental assumption (Claude CLI reads image paths from prompt text) should be validated before this ships; if it doesn't work that way, the approach needs rethinking entirely.
— Friday, AI assistant to @RichardAtCT
- Move uuid import to module scope (CLAUDE.md style requirement) - Validate image format before writing to disk; raise ValueError for unknown formats - Use restrictive permissions: dir 0o700, file 0o600 - Assign image_bytes once, remove redundant bytes() conversions - Explicitly instruct Claude to use Read tool for image path in all prompts - Add finally block in agentic_photo to delete temp file after run_command - Retain base64_data field with comment clarifying it is for future SDK support
|
Addressed all review feedback in 62f27f7:
491 passing, 0 failures. |
|
PR Review (follow-up) Previous issues — status
New observations
Verdict — Friday, AI assistant to @RichardAtCT |
|
PR Review (follow-up) Previous issues — status
New observations
Verdict — Friday, AI assistant to @RichardAtCT |
- Wrap write_bytes + chmod in try/except to unlink partial file on failure - Add type hints to all four _create_*_prompt methods (CLAUDE.md compliance)
|
Fixed in 48612f2:
491 passing. |
Problem
Image uploads were silently broken.
process_image()built a text promptsaying "I'm sharing a screenshot" but never actually passed the image to Claude.
The Claude CLI SDK only accepts text prompts — so the image was completely
unreachable and Claude would respond "I don't see any image attached."
Root Cause
ImageHandler.process_image()downloaded the image bytes and encoded themas base64, but
_handle_agentic_media_messageonly forwards aprompt: strto
claude_integration.run_command(). The image data was stored inProcessedImage.base64_databut never used.Fix
Download the image to
/tmp/claude_bot_files/image_<uuid>.<ext>and embedthe file path directly in the prompt so Claude CLI can read the image from disk.
All four prompt builders updated:
_create_screenshot_prompt,_create_diagram_prompt,_create_ui_prompt,_create_generic_prompt.Testing
Closes #137