Skip to content

fix(computer): send sandbox image downloads as images#7785

Merged
Soulter merged 1 commit intoAstrBotDevs:masterfrom
RhoninSeiei:codex/7783-sandbox-image-download-api
Apr 25, 2026
Merged

fix(computer): send sandbox image downloads as images#7785
Soulter merged 1 commit intoAstrBotDevs:masterfrom
RhoninSeiei:codex/7783-sandbox-image-download-api

Conversation

@RhoninSeiei
Copy link
Copy Markdown
Contributor

@RhoninSeiei RhoninSeiei commented Apr 24, 2026

背景

修复 #7783 中提到的问题:sandbox 内生成图片后,通过 astrbot_download_file 下载并发送到 OneBot/NapCat 时,会被 File 组件发送为文件消息,导致客户端显示为文件而非图片。

修改内容

本次修改在 astrbot_download_filealso_send_to_user 分支中按本地下载文件后缀选择消息组件:

  • pngjpgjpeggifbmpwebp 使用 Image.fromFileSystem() 发送。
  • 其他后缀继续使用原有 File 组件发送。
  • 如果发送阶段异常,工具返回中保留“文件已成功下载到本地临时目录”的信息,同时附带发送失败原因,方便排查适配器或 OneBot 端问题。

验证

  • 已检查提交差异,仅修改 astrbot/core/tools/computer_tools/fs.py
  • 修改保持原有下载逻辑、权限检查和非图片文件发送行为。

Fixes #7783

Checklist / 检查清单

  • 😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
    / 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。

  • 👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
    / 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”

  • 🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in requirements.txt and pyproject.toml.
    / 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到 requirements.txtpyproject.toml 文件相应位置。

  • 😮 My changes do not introduce malicious code.
    / 我的更改没有引入恶意代码。

Summary by Sourcery

Adjust sandbox file download behavior so that image files are sent as image messages instead of generic files when also sending to the user, while preserving existing behavior for non-image files and improving error reporting on send failures.

Bug Fixes:

  • Ensure images downloaded from the sandbox and sent via OneBot/NapCat are delivered as image messages rather than file attachments when using astrbot_download_file with also_send_to_user enabled.

Enhancements:

  • Return clearer status messages indicating whether the downloaded content was sent as an image or file and include detailed reasons when sending to the user fails.

@auto-assign auto-assign Bot requested review from Raven95676 and Soulter April 24, 2026 21:58
@dosubot dosubot Bot added size:S This PR changes 10-29 lines, ignoring generated files. area:core The bug / feature is about astrbot's core, backend labels Apr 24, 2026
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

Prompt for AI Agents
Please address the comments from this code review:

## Individual Comments

### Comment 1
<location path="astrbot/core/tools/computer_tools/fs.py" line_range="743-745" />
<code_context>
+                        MessageChain(chain=[message_component])
                     )
                 except Exception as e:
                     logger.error(f"Error sending file message: {e}")
+                    return (
+                        f"File downloaded successfully to {local_path} "
+                        f"but sending to user failed: {e}"
+                    )
</code_context>
<issue_to_address>
**🚨 issue (security):** Returning the full local path and raw exception message may expose internal details to the user.

Consider returning a user-friendly error and logging the full path/exception instead, or redacting sensitive parts of the path and exception before including them in the user-visible message.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment on lines 743 to +745
logger.error(f"Error sending file message: {e}")
return (
f"File downloaded successfully to {local_path} "
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Returning the full local path and raw exception message may expose internal details to the user.

Consider returning a user-friendly error and logging the full path/exception instead, or redacting sensitive parts of the path and exception before including them in the user-visible message.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the file system tool to distinguish between images and general files by checking file extensions, ensuring images are sent as image components. It also adds more descriptive error handling when message delivery fails. Feedback was provided to use os.path.splitext for consistency with existing code and to refactor the attachment logic into a shared helper function to avoid potential duplication.

Comment on lines +733 to +738
if Path(local_path).suffix.lower() in _IMAGE_FILE_SUFFIXES:
message_component = Image.fromFileSystem(local_path)
sent_as = "image"
else:
message_component = File(name=name, file=local_path)
sent_as = "file"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for determining the message component based on the file extension is correct. However, since os.path is already heavily used in this function (e.g., os.path.basename on line 732), you might consider using os.path.splitext(local_path)[1].lower() to avoid creating a Path object just for the suffix check, which would be slightly more efficient and consistent with the surrounding code. Additionally, if this logic for handling attachments is implemented similarly for different cases (e.g., direct vs. quoted attachments), please refactor it into a shared helper function to avoid code duplication.

                    suffix = os.path.splitext(local_path)[1].lower()
                    if suffix in _IMAGE_FILE_SUFFIXES:
                        message_component = Image.fromFileSystem(local_path)
                        sent_as = "image"
                    else:
                        message_component = File(name=name, file=local_path)
                        sent_as = "file"
References
  1. When implementing similar functionality for different cases (e.g., direct vs. quoted attachments), refactor the logic into a shared helper function to avoid code duplication.

Copy link
Copy Markdown
Contributor Author

补充验证:当前 head 726a2f6 的 Code Format Check、Unit Tests、CodeQL、Smoke Test、AstrBot Dashboard CI 均已通过。

@dosubot dosubot Bot added the lgtm This PR has been approved by a maintainer label Apr 25, 2026
@Soulter Soulter merged commit d4cdeea into AstrBotDevs:master Apr 25, 2026
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core The bug / feature is about astrbot's core, backend lgtm This PR has been approved by a maintainer size:S This PR changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] sandbox 下载图片发送到 OneBot/NapCat 时被当作文件消息

2 participants