Conversation
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
嘿——我发现了 3 个问题
面向 AI Agent 的提示
请处理这次代码审查中的评论:
## 单独评论
### 评论 1
<location path="crates/maa-cli/src/run/mod.rs" line_range="125" />
<code_context>
}
-fn run_core<F>(f: F, args: CommonArgs) -> Result<()>
+fn run_core<F>(f: F, args: CommonArgs, emit_json: bool) -> Result<()>
where
F: FnOnce(&AsstConfig) -> Result<TaskConfig>,
</code_context>
<issue_to_address>
**问题(更广泛的影响):** 对于会直接向 stdout 打印提示和表格的交互式预设命令,`--emit-json` 无法生成纯 JSONL stdout 流。例如,`Copilot` 和 `SSSCopilot` 在构建任务配置时会打印编队/阶段说明以及确认提示,而这些操作发生在发出任何 JSON 事件之前。因此,即使使用了 `--no-summary`,解析 stdout 的消费者仍会收到非 JSON 行。
**触发条件:** 对需要交互式设置的 `copilot` 或 `sss-copilot` 等预设使用 `--emit-json` 时。
**建议修复:** 启用 `emit_json` 时,将所有面向交互用户的输出转到 stderr;或者在 JSON 模式下拒绝这些交互式预设。
</issue_to_address>
### 评论 2
<location path="crates/maa-cli/src/run/callback/mod.rs" line_range="73" />
<code_context>
+///
+/// The event is timestamped here, at the moment it is emitted, from the same
+/// clock the human-readable log uses, so the two streams stay consistent.
+pub(crate) fn emit_json_event(kind: &str, message: &Value) {
+ if let Some(line) = format_event_line(kind, chrono::Local::now(), message) {
+ let mut out = std::io::stdout().lock();
+ // Ignore write errors: if stdout is closed (broken pipe), we silently
+ // drop the line rather than propagate an error up through a MaaCore
+ // callback thread or the task-registration loop.
+ let _ = writeln!(out, "{line}");
+ }
+}
</code_context>
<issue_to_address>
**问题(错误风险):** `emit_json_event` 会写入每条 JSONL 记录,但从不刷新 stdout。当 stdout 是管道时,标准输出缓冲区不一定会在每次 `writeln!` 后刷新,因此,程序化消费者无法及时收到事件,并且可能会一直阻塞等待回调,直到缓冲区填满或进程退出。
**触发条件:** 调用方通过管道增量消费 stdout,而不是在进程终止后读取时。
**建议修复:** 在每个事件后调用 `out.flush()`,并按照现有写入错误处理策略一致地处理刷新错误。
```suggestion
let _ = writeln!(out, "{line}");
let _ = out.flush();
```
</issue_to_address>
### 评论 3
<location path="crates/maa-cli/src/run/callback/mod.rs" line_range="56" />
<code_context>
+ // RFC 3339 with millisecond precision and a local UTC offset: parses
+ // unambiguously in every consumer, and keeps events emitted within the
+ // same second orderable by timestamp alone.
+ time: time.to_rfc3339_opts(chrono::SecondsFormat::Millis, false),
+ message,
+ };
</code_context>
<issue_to_address>
**小建议:** 毫秒级时间戳并不能保证同一秒内发出的事件仅凭时间戳就可以排序:多个回调可能会收到相同的毫秒级时间戳,并且系统墙上时钟的调整也可能导致连续的时间戳相同或倒退。文档和注释夸大了排序保证。
**触发条件:** 回调在同一毫秒内到达,或运行期间本地墙上时钟发生调整时。
**建议修复:** 将时间戳描述为近似的发出时间;如果消费者需要严格的事件顺序,则添加单调递增的序列号。
</issue_to_address>帮助我变得更有用!请在每条评论上点击 👍 或 👎,我会利用反馈来改进审查结果。
Original comment in English
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="crates/maa-cli/src/run/mod.rs" line_range="125" />
<code_context>
}
-fn run_core<F>(f: F, args: CommonArgs) -> Result<()>
+fn run_core<F>(f: F, args: CommonArgs, emit_json: bool) -> Result<()>
where
F: FnOnce(&AsstConfig) -> Result<TaskConfig>,
</code_context>
<issue_to_address>
**issue (broader_impact):** `--emit-json` does not produce a pure JSONL stdout stream for interactive preset commands that print prompts and tables directly to stdout. For example, `Copilot` and `SSSCopilot` print formation/stage instructions and confirmation prompts while constructing the task configuration, before any JSON event is emitted, so consumers parsing stdout receive non-JSON lines even with `--no-summary`.
**Triggers:** When using `--emit-json` with a preset such as `copilot` or `sss-copilot` that requires interactive setup.
**Suggested fix:** Route all interactive human-facing output to stderr when `emit_json` is enabled, or reject these interactive presets in JSON mode.
</issue_to_address>
### Comment 2
<location path="crates/maa-cli/src/run/callback/mod.rs" line_range="73" />
<code_context>
+///
+/// The event is timestamped here, at the moment it is emitted, from the same
+/// clock the human-readable log uses, so the two streams stay consistent.
+pub(crate) fn emit_json_event(kind: &str, message: &Value) {
+ if let Some(line) = format_event_line(kind, chrono::Local::now(), message) {
+ let mut out = std::io::stdout().lock();
+ // Ignore write errors: if stdout is closed (broken pipe), we silently
+ // drop the line rather than propagate an error up through a MaaCore
+ // callback thread or the task-registration loop.
+ let _ = writeln!(out, "{line}");
+ }
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** `emit_json_event` writes each JSONL record but never flushes stdout. When stdout is a pipe, the standard output buffer is not necessarily flushed after each `writeln!`, so programmatic consumers do not receive events promptly and can block waiting for callbacks until the buffer fills or the process exits.
**Triggers:** When a caller consumes stdout incrementally through a pipe rather than after process termination.
**Suggested fix:** Call `out.flush()` after each event, handling the flush error consistently with the existing write-error policy.
```suggestion
let _ = writeln!(out, "{line}");
let _ = out.flush();
```
</issue_to_address>
### Comment 3
<location path="crates/maa-cli/src/run/callback/mod.rs" line_range="56" />
<code_context>
+ // RFC 3339 with millisecond precision and a local UTC offset: parses
+ // unambiguously in every consumer, and keeps events emitted within the
+ // same second orderable by timestamp alone.
+ time: time.to_rfc3339_opts(chrono::SecondsFormat::Millis, false),
+ message,
+ };
</code_context>
<issue_to_address>
**nitpick:** The millisecond timestamp does not guarantee that events emitted within the same second are orderable by timestamp alone: multiple callbacks can receive the same millisecond timestamp, and wall-clock adjustments can also make successive timestamps equal or move backwards. The documentation and comments overstate the ordering guarantee.
**Triggers:** When callbacks arrive within one millisecond or the local wall clock is adjusted during a run.
**Suggested fix:** Describe the timestamp as an approximate emission time and add a monotonic sequence number if consumers require strict event ordering.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Open
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #579 +/- ##
==========================================
+ Coverage 72.36% 72.40% +0.03%
==========================================
Files 72 72
Lines 6804 6806 +2
Branches 6804 6806 +2
==========================================
+ Hits 4924 4928 +4
- Misses 1537 1583 +46
+ Partials 343 295 -48 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
wangl-cc
requested changes
Sep 12, 2026
| /// side effects of callback processing still run. Intended for programmatic | ||
| /// drivers, not for interactive use. | ||
| #[arg(long, global = true, verbatim_doc_comment)] | ||
| pub(crate) emit_json: bool, |
Member
There was a problem hiding this comment.
这个为什么不做成 run::CommonArgs? Core 只有 run 相关的命令才会加载。
| /// tests can assert on a fixed value. Returns `None` if the event fails to | ||
| /// serialize (should not happen in practice — `message` is a `Value` we just | ||
| /// built). | ||
| pub(crate) fn format_event_line( |
Member
There was a problem hiding this comment.
这个额外的序列化有点多余吧,Event 可以直接直接持有 MessageKind 和 time 没必要转成字符串。
MessageKind 当前没提供序列化,可以在 maa-types 里面加一下。
Member
|
我在想 “结构化事件输出” 这一节是否可以单独写在一个文档里?因为这算是高级用法。 |
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.
Summary
Adds a global
--emit-jsonflag that writes each MaaCore callback as one line of JSON on stdout, for callers driving maa-cli programmatically.This predates #574 by a week and implements the approach discussed there: forward Core's JSON as-is. On the channel concern raised in that thread, JSONL goes to stdout while the human-readable log stays on stderr, so
--emit-json --no-summarygives a consumer pure JSONL with nothing to filter.Changes
--emit-json: each callback becomes{"kind": "TaskChainStart", "time": ..., "message": {...}}, wheremessageis the raw MaaCore payloadTaskRegistered: a synthetic event per task before the run starts, carrying the registeredid,name,task_typeandparams;idmatches thetaskidin MaaCore's own chain callbacks, so a consumer can render the plan before MaaCore emits anythingtime: RFC 3339 with millisecond precision, since MaaCore payloads carry no time of their ownLeft as three commits. The second is the only one where maa-cli emits something MaaCore did not produce; if that is out of scope, dropping it leaves the other two working. Callback processing itself is unchanged, so the summary, exit status and report requests behave as before.
Validation
cargo +nightly fmt,cargo clippy,cargo test -p maa-cli(328 passed, 26 ignored)Notes
Docs updated in
en-USandzh-CNonly. Happy to follow whatever you prefer for the other three languages.Sourcery 摘要
为 maa-cli 提供机器可读的事件流,以便以编程方式执行任务,同时保持现有的面向用户行为不变。
新功能:
--emit-json选项,将 MaaCore 回调以带时间戳的 JSONL 格式输出到 stdout,供程序化使用者消费。TaskRegistered事件。增强:
文档:
测试:
Original summary in English
Sourcery 摘要
为 maa-cli 提供机器可读的事件流,以便程序化任务运行器使用,同时保留现有的面向用户行为。
新功能:
--emit-json选项,将 MaaCore 回调以带时间戳的 JSONL 格式输出到 stdout。TaskRegistered事件。增强:
文档:
测试:
Original summary in English
Summary by Sourcery
Provide maa-cli with a machine-readable event stream for programmatic task runners while preserving existing human-facing behavior.
New Features:
--emit-jsonoption that streams MaaCore callbacks as timestamped JSONL on stdout.TaskRegisteredevents containing resolved task metadata before execution begins.Enhancements:
Documentation:
Tests: