feat(sandbox): wait for Shipyard Neo readiness#7779
feat(sandbox): wait for Shipyard Neo readiness#7779RhoninSeiei wants to merge 1 commit intoAstrBotDevs:masterfrom
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- There are now two very similar helpers for exception formatting (
_format_exception_detailinshipyard_neo.pyandformat_exception_messageincomputer_tools/util.py); consider consolidating them into a single shared utility to avoid divergence in behavior. - In
_wait_until_shell_readyyou catch a broadExceptionfromself._shell.exec; if there are known non-retryable error types (e.g., auth or 4xx/5xx errors from the backend), it may be worth distinguishing them to fail fast rather than continuing to poll.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There are now two very similar helpers for exception formatting (`_format_exception_detail` in `shipyard_neo.py` and `format_exception_message` in `computer_tools/util.py`); consider consolidating them into a single shared utility to avoid divergence in behavior.
- In `_wait_until_shell_ready` you catch a broad `Exception` from `self._shell.exec`; if there are known non-retryable error types (e.g., auth or 4xx/5xx errors from the backend), it may be worth distinguishing them to fail fast rather than continuing to poll.
## Individual Comments
### Comment 1
<location path="astrbot/core/computer/booters/shipyard_neo.py" line_range="39-41" />
<code_context>
return {}
+def _format_exception_detail(exc: BaseException) -> str:
+ message = str(exc).strip()
+ return message or exc.__class__.__name__
+
+
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating exception-formatting logic that already exists in `computer_tools.util.format_exception_message`.
This helper duplicates `format_exception_message` in `computer_tools/util.py`. Prefer reusing that utility (or moving this logic there) so exception formatting stays consistent and only needs to be updated in one place.
Suggested implementation:
```python
from astrbot.api import logger
from computer_tools.util import format_exception_message
return {}
```
1. Remove the `_format_exception_detail` definition from `astrbot/core/computer/booters/shipyard_neo.py`:
- Delete:
```python
def _format_exception_detail(exc: BaseException) -> str:
message = str(exc).strip()
return message or exc.__class__.__name__
```
2. Replace any calls to `_format_exception_detail(exc)` in this file with `format_exception_message(exc)`:
- For example, change:
```python
detail = _format_exception_detail(exc)
```
to:
```python
detail = format_exception_message(exc)
```
3. If there are other modules that copied this helper, they should similarly import and use `format_exception_message` from `computer_tools.util` to keep exception formatting consistent project-wide.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def _format_exception_detail(exc: BaseException) -> str: | ||
| message = str(exc).strip() | ||
| return message or exc.__class__.__name__ |
There was a problem hiding this comment.
suggestion: Avoid duplicating exception-formatting logic that already exists in computer_tools.util.format_exception_message.
This helper duplicates format_exception_message in computer_tools/util.py. Prefer reusing that utility (or moving this logic there) so exception formatting stays consistent and only needs to be updated in one place.
Suggested implementation:
from astrbot.api import logger
from computer_tools.util import format_exception_message
return {}-
Remove the
_format_exception_detaildefinition fromastrbot/core/computer/booters/shipyard_neo.py:- Delete:
def _format_exception_detail(exc: BaseException) -> str: message = str(exc).strip() return message or exc.__class__.__name__
-
Replace any calls to
_format_exception_detail(exc)in this file withformat_exception_message(exc):- For example, change:
detail = _format_exception_detail(exc)
to:
detail = format_exception_message(exc)
-
If there are other modules that copied this helper, they should similarly import and use
format_exception_messagefromcomputer_tools.utilto keep exception formatting consistent project-wide.
There was a problem hiding this comment.
Code Review
This pull request introduces a shell readiness check for the Shipyard Neo booter, ensuring that the shell is available before proceeding with tool synchronization. It adds utility functions for formatting exception and shell result details, which are now used across various computer tools to provide more informative error messages. Additionally, new unit tests have been added to verify the shell readiness retry logic and timeout behavior. I have no feedback to provide.
Fixes #7777
Modifications / 改动点
本 PR 修复 Shipyard Neo 冷启动期间首次 sandbox 工具调用容易因
httpx.ReadTimeout返回空error:的问题。在
ShipyardNeoBooter.boot()中,create_sandbox()成功后增加 shell 就绪探测,遇到冷启动阶段的临时超时会等待并重试,直到 shell 可执行后再进入后续 skills 同步与工具调用。readiness 等待保留 90 秒总上限,超过上限时抛出包含 sandbox id、profile 和最后错误原因的
TimeoutError,避免长时间无响应。增加通用异常文本格式化函数,当异常字符串为空时使用异常类型名,例如
ReadTimeout或TimeoutError,避免工具侧只显示空错误。shell、python、Shipyard Neo browser 相关工具复用该异常文本格式化逻辑。
增加单元测试覆盖冷启动第一次超时后重试成功,以及空
TimeoutError能正确显示异常类型。This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
已执行以下验证:
真实 Shipyard Neo 环境验证:冷启动后 readiness 探测先捕获
ReadTimeout,随后第 3 次探测成功,后续 shell 与 python 调用均成功:{ "shell": { "stdout": "prod14-ready\n/workspace", "stderr": "", "exit_code": 0, "success": true }, "python": { "success": true, "output": "17", "error": "" } }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.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Add readiness checks for Shipyard Neo sandboxes and improve error reporting for computer tools.
New Features:
Bug Fixes:
Enhancements:
Tests: