chore: development to master #1490
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| temp_file.close() | ||
|
|
||
| logger.debug(f"Saving temp file to {temp_path}") | ||
| image.save(temp_path) # PIL needs a file to copy to clipboard |
There was a problem hiding this comment.
Potential resource leak: The image object is not being closed after use. Consider using a context manager or explicitly calling image.close() after saving to prevent memory leaks, especially when handling large images.
| """Type definition for clipboard state.""" | ||
|
|
||
| text_data: str | ||
| image_data: str |
There was a problem hiding this comment.
Consider using bytes type for image_data instead of str since it's storing binary image data that's base64 encoded. This would make the type hint more accurate and explicit about the expected data type.
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| class ClipboardState(TypedDict, total=False): | ||
| """Type definition for clipboard state.""" | ||
|
|
||
| text_data: str | ||
| image_data: str | ||
| file_paths: list[str] |
There was a problem hiding this comment.
The ClipboardState TypedDict uses total=False but doesn't handle the case where keys might be missing when accessed. This could lead to KeyError exceptions when trying to access non-existent keys.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| class ClipboardState(TypedDict, total=False): | |
| """Type definition for clipboard state.""" | |
| text_data: str | |
| image_data: str | |
| file_paths: list[str] | |
| class ClipboardState(TypedDict, total=False): | |
| """Type definition for clipboard state.""" | |
| text_data: Optional[str] = None | |
| image_data: Optional[str] = None | |
| file_paths: Optional[list[str]] = None |
| def get_clipboard_state(metadata: Dict[str, Any]) -> ClipboardState: | ||
| """Get clipboard state from metadata. | ||
|
|
||
| Args: | ||
| metadata: The metadata dictionary containing clipboard state | ||
|
|
||
| Returns: | ||
| The clipboard state dictionary, initialized if it doesn't exist | ||
| """ | ||
| if "clipboard_state" not in metadata: | ||
| metadata["clipboard_state"] = {} | ||
| return metadata["clipboard_state"] # type: ignore |
There was a problem hiding this comment.
The get_clipboard_state function uses # type: ignore to suppress a type error instead of properly typing the return value. The function should explicitly cast the return value to ClipboardState.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| def get_clipboard_state(metadata: Dict[str, Any]) -> ClipboardState: | |
| """Get clipboard state from metadata. | |
| Args: | |
| metadata: The metadata dictionary containing clipboard state | |
| Returns: | |
| The clipboard state dictionary, initialized if it doesn't exist | |
| """ | |
| if "clipboard_state" not in metadata: | |
| metadata["clipboard_state"] = {} | |
| return metadata["clipboard_state"] # type: ignore | |
| def get_clipboard_state(metadata: Dict[str, Any]) -> ClipboardState: | |
| """Get clipboard state from metadata. | |
| Args: | |
| metadata: The metadata dictionary containing clipboard state | |
| Returns: | |
| The clipboard state dictionary, initialized if it doesn't exist | |
| """ | |
| if "clipboard_state" not in metadata: | |
| metadata["clipboard_state"] = {} | |
| return ClipboardState(metadata["clipboard_state"]) |
| image = Image.open(request.image_path) | ||
| temp_file = tempfile.NamedTemporaryFile(delete=False, suffix=".png") | ||
| temp_path = temp_file.name | ||
| temp_file.close() | ||
|
|
||
| logger.debug(f"Saving temp file to {temp_path}") | ||
| image.save(temp_path) # PIL needs a file to copy to clipboard |
There was a problem hiding this comment.
The CopyImage action creates a temporary PNG file regardless of the original image format, which could cause quality loss for certain image types. Should preserve the original format.
| # Decode and save image | ||
| data = base64.b64decode(image_data) | ||
| with open(request.save_path, "wb") as f: | ||
| f.write(data) |
There was a problem hiding this comment.
The code doesn't validate the image format when pasting. It should check if the decoded data is a valid image before writing to the file system.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| # Decode and save image | |
| data = base64.b64decode(image_data) | |
| with open(request.save_path, "wb") as f: | |
| f.write(data) | |
| # Decode and validate image before saving | |
| data = base64.b64decode(image_data) | |
| try: | |
| # Validate image format using PIL | |
| Image.open(io.BytesIO(data)).verify() | |
| with open(request.save_path, "wb") as f: | |
| f.write(data) | |
| except Exception as e: | |
| raise ValueError(f"Invalid image format: {str(e)}") |
| import os | ||
| # Disable remote enum fetching | ||
| os.environ["COMPOSIO_NO_REMOTE_ENUM_FETCHING"] = "true" | ||
|
|
||
| from composio.client.enums.base import add_runtime_action, ActionData | ||
| from composio.tools.local.clipboardtool.actions.files import CopyFilePaths, PasteFilePaths | ||
| from composio.tools.local.clipboardtool.actions.image import CopyImage, PasteImage | ||
| from composio.tools.local.clipboardtool.actions.text import CopyText, PasteText | ||
| from composio.tools.local.clipboardtool.tool import Clipboardtool | ||
| from composio.tools.base.abs import action_registry | ||
|
|
||
| def register_clipboard_actions(): | ||
| """Register clipboard actions in the Action enum.""" | ||
| actions = [ | ||
| (CopyText, "Copy text to clipboard"), | ||
| (PasteText, "Paste text from clipboard"), | ||
| (CopyImage, "Copy image to clipboard"), | ||
| (PasteImage, "Paste image from clipboard"), | ||
| (CopyFilePaths, "Copy file paths to clipboard"), | ||
| (PasteFilePaths, "Paste file paths from clipboard"), | ||
| ] | ||
|
|
||
| for action, _ in actions: | ||
| add_runtime_action( | ||
| f"CLIPBOARDTOOL_{action.__name__.upper()}", | ||
| ActionData( | ||
| name=action.__name__, | ||
| app="CLIPBOARDTOOL", | ||
| tags=[], | ||
| no_auth=True, | ||
| is_local=True, | ||
| is_runtime=True, | ||
| ) | ||
| ) | ||
|
|
||
| # Register actions before importing Action | ||
| register_clipboard_actions() | ||
|
|
||
| from composio import Action, ComposioToolSet | ||
| from PIL import Image | ||
| import os |
There was a problem hiding this comment.
Duplicate import of os module at lines 1 and 41. The second import is redundant and should be removed.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| import os | |
| # Disable remote enum fetching | |
| os.environ["COMPOSIO_NO_REMOTE_ENUM_FETCHING"] = "true" | |
| from composio.client.enums.base import add_runtime_action, ActionData | |
| from composio.tools.local.clipboardtool.actions.files import CopyFilePaths, PasteFilePaths | |
| from composio.tools.local.clipboardtool.actions.image import CopyImage, PasteImage | |
| from composio.tools.local.clipboardtool.actions.text import CopyText, PasteText | |
| from composio.tools.local.clipboardtool.tool import Clipboardtool | |
| from composio.tools.base.abs import action_registry | |
| def register_clipboard_actions(): | |
| """Register clipboard actions in the Action enum.""" | |
| actions = [ | |
| (CopyText, "Copy text to clipboard"), | |
| (PasteText, "Paste text from clipboard"), | |
| (CopyImage, "Copy image to clipboard"), | |
| (PasteImage, "Paste image from clipboard"), | |
| (CopyFilePaths, "Copy file paths to clipboard"), | |
| (PasteFilePaths, "Paste file paths from clipboard"), | |
| ] | |
| for action, _ in actions: | |
| add_runtime_action( | |
| f"CLIPBOARDTOOL_{action.__name__.upper()}", | |
| ActionData( | |
| name=action.__name__, | |
| app="CLIPBOARDTOOL", | |
| tags=[], | |
| no_auth=True, | |
| is_local=True, | |
| is_runtime=True, | |
| ) | |
| ) | |
| # Register actions before importing Action | |
| register_clipboard_actions() | |
| from composio import Action, ComposioToolSet | |
| from PIL import Image | |
| import os | |
| import os | |
| # Disable remote enum fetching | |
| os.environ["COMPOSIO_NO_REMOTE_ENUM_FETCHING"] = "true" | |
| from composio.client.enums.base import add_runtime_action, ActionData | |
| from composio.tools.local.clipboardtool.actions.files import CopyFilePaths, PasteFilePaths | |
| from composio.tools.local.clipboardtool.actions.image import CopyImage, PasteImage | |
| from composio.tools.local.clipboardtool.actions.text import CopyText, PasteText | |
| from composio.tools.local.clipboardtool.tool import Clipboardtool | |
| from composio.tools.base.abs import action_registry | |
| def register_clipboard_actions(): | |
| """Register clipboard actions in the Action enum.""" | |
| actions = [ | |
| (CopyText, "Copy text to clipboard"), | |
| (PasteText, "Paste text from clipboard"), | |
| (CopyImage, "Copy image to clipboard"), | |
| (PasteImage, "Paste image from clipboard"), | |
| (CopyFilePaths, "Copy file paths to clipboard"), | |
| (PasteFilePaths, "Paste file paths from clipboard"), | |
| ] | |
| for action, _ in actions: | |
| add_runtime_action( | |
| f"CLIPBOARDTOOL_{action.__name__.upper()}", | |
| ActionData( | |
| name=action.__name__, | |
| app="CLIPBOARDTOOL", | |
| tags=[], | |
| no_auth=True, | |
| is_local=True, | |
| is_runtime=True, | |
| ) | |
| ) | |
| # Register actions before importing Action | |
| register_clipboard_actions() | |
| from composio import Action, ComposioToolSet | |
| from PIL import Image |
| def register_clipboard_actions(): | ||
| """Register clipboard actions in the Action enum.""" | ||
| actions = [ | ||
| (CopyText, "Copy text to clipboard"), | ||
| (PasteText, "Paste text from clipboard"), | ||
| (CopyImage, "Copy image to clipboard"), | ||
| (PasteImage, "Paste image from clipboard"), | ||
| (CopyFilePaths, "Copy file paths to clipboard"), | ||
| (PasteFilePaths, "Paste file paths from clipboard"), | ||
| ] | ||
|
|
||
| for action, _ in actions: | ||
| add_runtime_action( | ||
| f"CLIPBOARDTOOL_{action.__name__.upper()}", | ||
| ActionData( | ||
| name=action.__name__, | ||
| app="CLIPBOARDTOOL", | ||
| tags=[], | ||
| no_auth=True, | ||
| is_local=True, | ||
| is_runtime=True, | ||
| ) | ||
| ) | ||
|
|
||
| # Register actions before importing Action | ||
| register_clipboard_actions() | ||
|
|
||
| from composio import Action, ComposioToolSet | ||
| from PIL import Image | ||
| import os | ||
| import tempfile | ||
|
|
||
| def main(): | ||
| # Load clipboard tool to register actions | ||
| tool = Clipboardtool() | ||
| # Register actions | ||
| tool.register() | ||
| actions = tool.actions() | ||
|
|
||
| # Register actions in action registry | ||
| for action in actions: | ||
| action_registry["runtime"][f"CLIPBOARDTOOL_{action.__name__.upper()}"] = action() |
There was a problem hiding this comment.
Redundant action registration. Actions are registered twice: once in register_clipboard_actions() and again in the main() function, which could cause unexpected behavior.
| """Execute the action.""" | ||
| try: | ||
| # Validate paths exist | ||
| valid_paths = [p for p in request.paths if os.path.exists(p)] |
There was a problem hiding this comment.
Consider adding path validation to prevent directory traversal attacks. You might want to use os.path.abspath() and ensure the paths don't escape a designated safe directory.
Code Review SummaryOverall, this is a well-structured and comprehensive implementation of a clipboard tool. Here's my assessment: Strengths:✅ Clean architecture with proper separation of concerns Areas for Improvement:
Rating: 8/10The code is production-ready with minor improvements needed. The suggestions above are mostly enhancements rather than critical issues. |
No description provided.