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. |
|
Closing in favor of #168, which sends images as multimodal SDK content blocks (base64 + media type) rather than file paths in prompts. The SDK multimodal approach is the proper integration path for Claude. Thank you for diagnosing the root cause of the image upload issue — your analysis was valuable. |
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