Repository navigation
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #630 +/- ##
==========================================
- Coverage 76.81% 76.33% -0.49%
==========================================
Files 74 75 +1
Lines 7763 7816 +53
Branches 7763 7816 +53
==========================================
+ Hits 5963 5966 +3
- Misses 1472 1522 +50
Partials 328 328 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿——我发现了 4 个问题
给 AI Agent 的提示
请处理这次代码审查中的评论：
## 单条评论
### 评论 1
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="65" />
<code_context>
+ // The blocking API can return before its completion callback is delivered.
+ loop {
+ let result = results
+ .recv()
+ .context("Screenshot callback disconnected")??;
+ if result.async_call_id == id {
</code_context>
<issue_to_address>
**截图命令会永久挂起**
当 maaCore 接受截图请求但始终不发送完成回调时，`wait_for_screencap` 会无限期阻塞在 `results.recv()` 上，因此截图命令始终不会返回，调用方也收不到错误。
添加有时限的等待或取消机制，以便在缺少完成回调时返回错误。
</issue_to_address>
### 评论 2
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="76" />
<code_context>
+
+fn save_image(output: &Path, image: Option<&[u8]>) -> Result<()> {
+ let image = image.context("No screenshot returned by MaaCore")?;
+ maa_atomic_fs::write(output, image)
+ .with_context(|| format!("Failed to save screenshot to {}", output.display()))
+}
</code_context>
<issue_to_address>
**较早完成的截图会覆盖较新的截图**
当两个截图命令写入同一个输出路径，且较早发起的截图比之后发起的截图更晚完成时，`save_image` 会以原子方式替换目标文件，却没有协调并发调用。因此，较早发起的截图可能最后完成并覆盖较新的截图；此时文件中保存的是过时的屏幕内容。
按目标路径串行化写入并保持调用顺序，或者要求调用方在并发截图时使用不同的输出路径。
</issue_to_address>
### 评论 3
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="92" />
<code_context>
+ let (callback, results) = screenshot_callback();
+ callback.on_message(
+ MessageKind::AsyncCallInfo,
+ Some(r#"{"what":"Connect","async_call_id":1,"details":{"ret":false}}"#),
+ );
+ std::thread::scope(|scope| {
</code_context>
<issue_to_address>
**无关的成功事件会结束截图等待**
当截图回调到达之前，先成功收到一个无关的 `AsyncCallInfo` 时，`crates/maa-cli/src/run/screenshot.rs:92` 中的 `waits_for_matching_result` 会将不匹配事件的 `ret` 设为 `false`。因此，如果等待逻辑接受任何成功回调而不检查其 ID，测试仍会通过，并在截图完成前返回。
在测试样例中使用一个成功但不匹配的事件，并断言只有在匹配的回调到达后等待逻辑才会完成。
</issue_to_address>
### 评论 4
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="101" />
<code_context>
+ Some(r#"{"what":"Screencap","async_call_id":2,"details":{"ret":true}}"#),
+ );
+ // The callback and receiver must finish without panicking.
+ completion.join().expect("screenshot receiver panicked")
+ })
+ }
</code_context>
<issue_to_address>
**测试中的错误处理缺乏合理说明**
新增测试调用了 `expect` 和 `unwrap_err`，但没有附上简短的理由说明，因此新增代码违反了仓库的错误处理规则。
请将这些调用替换为显式的结果处理，或分别为每个调用添加简短的理由说明。
`crates/maa-cli/src/run/screenshot.rs:159` 处也有此问题。
</issue_to_address>Original comment in English
Hey - I've found 4 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="65" />
<code_context>
+ // The blocking API can return before its completion callback is delivered.
+ loop {
+ let result = results
+ .recv()
+ .context("Screenshot callback disconnected")??;
+ if result.async_call_id == id {
</code_context>
<issue_to_address>
**Screenshot commands hang forever**
When maaCore accepts a screenshot request but never delivers its completion callback, `wait_for_screencap` blocks on `results.recv()` indefinitely, so the screenshot command never returns and the caller receives no error.
Add a bounded wait or cancellation path so a missing completion callback returns an error.
</issue_to_address>
### Comment 2
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="76" />
<code_context>
+
+fn save_image(output: &Path, image: Option<&[u8]>) -> Result<()> {
+ let image = image.context("No screenshot returned by MaaCore")?;
+ maa_atomic_fs::write(output, image)
+ .with_context(|| format!("Failed to save screenshot to {}", output.display()))
+}
</code_context>
<issue_to_address>
**Older capture overwrites newer screenshot**
When two screenshot commands write to the same output path and the older capture finishes after the newer one, `save_image` atomically replaces the destination without coordinating concurrent invocations, so the older capture can finish last and replace a newer screenshot; the file then contains stale screen content.
Serialize writes per destination and preserve invocation order, or require callers to use distinct output paths for concurrent captures.
</issue_to_address>
### Comment 3
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="92" />
<code_context>
+ let (callback, results) = screenshot_callback();
+ callback.on_message(
+ MessageKind::AsyncCallInfo,
+ Some(r#"{"what":"Connect","async_call_id":1,"details":{"ret":false}}"#),
+ );
+ std::thread::scope(|scope| {
</code_context>
<issue_to_address>
**Unrelated success completes screenshot wait**
When an unrelated `AsyncCallInfo` arrives successfully before the screencap callback, `waits_for_matching_result` at `crates/maa-cli/src/run/screenshot.rs:92` makes its mismatched event `ret:false`, so a waiter that accepts any successful callback without checking its ID still passes and returns before capture completes.
Use a successful nonmatching event in the fixture and assert the waiter completes only after the matching callback arrives.
</issue_to_address>
### Comment 4
<location path="crates/maa-cli/src/run/screenshot.rs" line_range="101" />
<code_context>
+ Some(r#"{"what":"Screencap","async_call_id":2,"details":{"ret":true}}"#),
+ );
+ // The callback and receiver must finish without panicking.
+ completion.join().expect("screenshot receiver panicked")
+ })
+ }
</code_context>
<issue_to_address>
**Unjustified test error handling**
The new tests call `expect` and `unwrap_err` without brief justification comments, so the added code violates the repository’s error-handling rule.
Replace these calls with explicit result handling, or add a brief justification comment for each.
Also at `crates/maa-cli/src/run/screenshot.rs:159`.
</issue_to_address>|
|
||
| fn save_image(output: &Path, image: Option<&[u8]>) -> Result<()> { | ||
| let image = image.context("No screenshot returned by MaaCore")?; | ||
| maa_atomic_fs::write(output, image) |
There was a problem hiding this comment.
🟡 中 · 较早完成的截图会覆盖较新的截图
当两个截图命令写入同一个输出路径，且较早发起的截图比之后发起的截图更晚完成时，save_image 会以原子方式替换目标文件，却没有协调并发调用。因此，较早发起的截图可能最后完成并覆盖较新的截图；此时文件中保存的是过时的屏幕内容。
按目标路径串行化写入并保持调用顺序，或者要求调用方在并发截图时使用不同的输出路径。
给 AI Agent 的提示
在 `crates/maa-cli/src/run/screenshot.rs` 第 76 行：
**较早完成的截图会覆盖较新的截图**
当两个截图命令写入同一个输出路径，且较早发起的截图比之后发起的截图更晚完成时，`save_image` 会以原子方式替换目标文件，却没有协调并发调用。因此，较早发起的截图可能最后完成并覆盖较新的截图；此时文件中保存的是过时的屏幕内容。
按目标路径串行化写入并保持调用顺序，或者要求调用方在并发截图时使用不同的输出路径。Original comment in English
🟡 Medium · Older capture overwrites newer screenshot
When two screenshot commands write to the same output path and the older capture finishes after the newer one, save_image atomically replaces the destination without coordinating concurrent invocations, so the older capture can finish last and replace a newer screenshot; the file then contains stale screen content.
Serialize writes per destination and preserve invocation order, or require callers to use distinct output paths for concurrent captures.
Prompt for AI agents
In `crates/maa-cli/src/run/screenshot.rs` at line 76:
**Older capture overwrites newer screenshot**
When two screenshot commands write to the same output path and the older capture finishes after the newer one, `save_image` atomically replaces the destination without coordinating concurrent invocations, so the older capture can finish last and replace a newer screenshot; the file then contains stale screen content.
Serialize writes per destination and preserve invocation order, or require callers to use distinct output paths for concurrent captures.
wangl-cc
marked this pull request as draft
October 24, 2026 15:17
Restore existing core capture behavior and document that failed refreshes can return cached frames.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
动机 / Motivation
增加
maa screenshot [output]，通过 MaaCore 获取设备画面，方便排查连接和画面问题。指定路径时保存 PNG；省略路径时将 PNG 原始字节写入 stdout，便于重定向或管道处理。方案 / Solution
复用任务的初始化和连接流程，支持
--profile、--addr和--user-resource。调用现有get_fresh_image()等待截图执行后读取缓存，不增加回调等待设施或改变 Assistant 的回调行为。连接成功检查统一放在with_connection；文件输出使用原子替换，stdout 输出二进制 PNG，日志默认走 stderr。截图不触发资源更新，也不启停游戏或执行任务队列。影响与验证 / Impact and testing
maa-core无默认 feature 编译检查，以及cargo x test --no-core-tests --no-ignored-tests通过。AI 辅助 / AI assistance
使用 Codex 编写实现、测试和文档。
Sourcery 摘要
新增截图子命令，通过 MaaCore 获取设备图像，并将其输出为 PNG 文件或标准输出。
新功能：
maa screenshot [output]命令，可捕获已连接设备的图像，并将 PNG 数据写入文件或标准输出。增强：
文档：
测试：
Original summary in English
Summary by Sourcery
Add a screenshot subcommand that retrieves device images through MaaCore and outputs them as PNG files or standard output.
New Features:
maa screenshot [output]command that captures a connected device image and writes PNG data to a file or stdout.Enhancements:
Documentation:
Tests: