feat(pi): 支持 MaaFramework v5.13.0 - #13
Conversation
按任务适用的控制器和资源校验并补全选项,避免清理或生成非活动任务的子树;同时补充空 checkbox 选择提示。
自动补全 Input 默认值,恢复失效运行时选项并过滤不适用任务;同时防御异常配置和 Linux 控制器创建失败。
There was a problem hiding this comment.
嘿——我发现了 3 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论:
## 个别评论
### 评论 1
<location path="Impl/Runner.cpp" line_range="443-458" />
<code_context>
+ obj["pw_node_id"] = pw_node_id;
+ }
+ else {
+ auto helper_handle = MaaToolkitPortalHelperCreate();
+ if (!helper_handle) {
+ LogError << "Failed to create portal helper";
+ return "";
+ }
+
+ if (!MaaToolkitPortalHelperOpenStream(helper_handle)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** 在 PipeWire 文件描述符和节点 ID 被复制到 JSON 后、`MaaLinuxControllerCreate` 使用该配置之前,Portal helper 就会被立即销毁。销毁 helper 会关闭或使由流支持的文件描述符失效,因此基于 Portal 的 Linux 控制器创建过程会收到过期的文件描述符,并失败或无法捕获屏幕。
**触发条件:** Linux 控制器使用 `screencap: "PipeWire"` 且 `pipewire_source: "Portal"` 时。
**建议修复:** 让 Portal helper 一直存活到 Linux 控制器使用完该流之后,或者在销毁 helper 之前转移/复制文件描述符。
</issue_to_address>
### 评论 2
<location path="CLI/interactor.cpp" line_range="2437-2451" />
<code_context>
+ auto existing_option_iter =
</code_context>
<issue_to_address>
**issue (bug_risk):** 现有的 Input 选项被复制到重建的选项树中,但没有填充缺失的已声明输入。当接口新增 Input 字段,或保存的选项中没有某个现有字段的值时,自动补全路径会使该输入缺失,而不是存储其声明的默认值。
**触发条件:** 保存的配置已经包含 Input 选项,但缺少当前接口声明的一个或多个输入时。
**建议修复:** 在保留现有值的同时,使用已声明的默认值补全缺失的 Input 条目,包括 `process_option` 使用的密码存储路径。
</issue_to_address>
### 评论 3
<location path="Impl/Parser.cpp" line_range="212-214" />
<code_context>
+ return false;
+ }
+
+ if (auto* defaults = std::get_if<std::vector<std::string>>(&option.default_case)) {
+ return checkbox_selection_is_valid(option, *defaults);
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** 当有效名称的数量满足边界条件时,包含未知 case 名称的复选框默认值会被接受,因为 `checkbox_selection_is_valid` 只统计已知 case,却从不拒绝未知值。因此接口可以成功解析,自动补全会静默丢弃无效默认值,而不是拒绝格式错误的定义。
**触发条件:** 复选框的 `default_case` 数组包含未知 case 名称,且有效名称数量仍满足 `min_count`/`max_count` 时(例如没有最小数量限制)。
**建议修复:** 在应用数量约束之前,针对 `option.cases` 验证每个默认值;如果任何默认名称未知,则拒绝该定义。
```suggestion
if (auto* defaults = std::get_if<std::vector<std::string>>(&option.default_case)) {
for (const auto& value : *defaults) {
if (std::ranges::find(option.cases, value, std::mem_fn(&InterfaceData::Option::Case::name)) == option.cases.end()) {
return false;
}
}
return checkbox_selection_is_valid(option, *defaults);
}
```
</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="Impl/Runner.cpp" line_range="443-458" />
<code_context>
+ obj["pw_node_id"] = pw_node_id;
+ }
+ else {
+ auto helper_handle = MaaToolkitPortalHelperCreate();
+ if (!helper_handle) {
+ LogError << "Failed to create portal helper";
+ return "";
+ }
+
+ if (!MaaToolkitPortalHelperOpenStream(helper_handle)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** The Portal helper is destroyed immediately after its PipeWire file descriptor and node ID are copied into the JSON, before `MaaLinuxControllerCreate` consumes that configuration. Destroying the helper closes or invalidates the stream-backed file descriptor, so Portal-based Linux controller creation receives a stale descriptor and fails or cannot capture the screen.
**Triggers:** When the Linux controller uses `screencap: "PipeWire"` with `pipewire_source: "Portal"`.
**Suggested fix:** Keep the Portal helper alive until after the Linux controller has consumed the stream, or transfer/duplicate the file descriptor before destroying the helper.
</issue_to_address>
### Comment 2
<location path="CLI/interactor.cpp" line_range="2437-2451" />
<code_context>
+ auto existing_option_iter =
</code_context>
<issue_to_address>
**issue (bug_risk):** An existing Input option is copied into the rebuilt option tree without filling missing declared inputs. When the interface adds a new Input field, or the saved option contains no value for an existing field, the automatic completion path leaves that input absent instead of storing its declared default.
**Triggers:** When a saved configuration already contains the Input option but is missing one or more inputs declared by the current interface.
**Suggested fix:** Complete missing Input entries with their declared defaults while preserving existing values, including the password-storage path used by `process_option`.
</issue_to_address>
### Comment 3
<location path="Impl/Parser.cpp" line_range="212-214" />
<code_context>
+ return false;
+ }
+
+ if (auto* defaults = std::get_if<std::vector<std::string>>(&option.default_case)) {
+ return checkbox_selection_is_valid(option, *defaults);
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Checkbox defaults containing unknown case names are accepted whenever the count of valid names satisfies the bounds, because `checkbox_selection_is_valid` counts only known cases but never rejects unknown values. The interface therefore parses successfully and automatic completion silently drops the invalid default instead of rejecting the malformed definition.
**Triggers:** When a checkbox `default_case` array contains an unknown case name and its valid-name count still satisfies `min_count`/`max_count` (for example, no minimum count).
**Suggested fix:** Validate every default value against `option.cases` before applying the count constraint, and reject the definition if any default name is unknown.
```suggestion
if (auto* defaults = std::get_if<std::vector<std::string>>(&option.default_case)) {
for (const auto& value : *defaults) {
if (std::ranges::find(option.cases, value, std::mem_fn(&InterfaceData::Option::Case::name)) == option.cases.end()) {
return false;
}
}
return checkbox_selection_is_valid(option, *defaults);
}
```
</issue_to_address>| auto helper_handle = MaaToolkitPortalHelperCreate(); | ||
| if (!helper_handle) { | ||
| LogError << "Failed to create portal helper"; | ||
| return ""; | ||
| } | ||
|
|
||
| if (!MaaToolkitPortalHelperOpenStream(helper_handle)) { | ||
| LogError << "Failed to open PipeWire stream"; | ||
| MaaToolkitPortalHelperDestroy(helper_handle); | ||
| return ""; | ||
| } | ||
|
|
||
| obj["pw_socket_fd"] = MaaToolkitPortalHelperGetPipeWireFD(helper_handle); | ||
| obj["pw_node_id"] = MaaToolkitPortalHelperGetPipeWireNodeID(helper_handle); | ||
| MaaToolkitPortalHelperDestroy(helper_handle); | ||
| } |
There was a problem hiding this comment.
issue (bug_risk): 在 PipeWire 文件描述符和节点 ID 被复制到 JSON 后、MaaLinuxControllerCreate 使用该配置之前,Portal helper 就会被立即销毁。销毁 helper 会关闭或使由流支持的文件描述符失效,因此基于 Portal 的 Linux 控制器创建过程会收到过期的文件描述符,并失败或无法捕获屏幕。
触发条件: Linux 控制器使用 screencap: "PipeWire" 且 pipewire_source: "Portal" 时。
建议修复: 让 Portal helper 一直存活到 Linux 控制器使用完该流之后,或者在销毁 helper 之前转移/复制文件描述符。
Original comment in English
issue (bug_risk): The Portal helper is destroyed immediately after its PipeWire file descriptor and node ID are copied into the JSON, before MaaLinuxControllerCreate consumes that configuration. Destroying the helper closes or invalidates the stream-backed file descriptor, so Portal-based Linux controller creation receives a stale descriptor and fails or cannot capture the screen.
Triggers: When the Linux controller uses screencap: "PipeWire" with pipewire_source: "Portal".
Suggested fix: Keep the Portal helper alive until after the Linux controller has consumed the stream, or transfer/duplicate the file descriptor before destroying the helper.
|
@sourcery-ai review |
Sourcery 摘要
将 MaaPiCli 升级至 MaaFramework v5.13.0,支持 Linux 控制器、扩展 PI 选项处理,并增强配置迁移与验证的可靠性。
新功能:
display_expand截图尺寸设置和 Win32 AnchoredTouch 输入配置。min_count和max_count约束,并处理空选择情况。Bug 修复:
增强:
文档:
测试:
Original summary in English
Sourcery 摘要
将 MaaPiCli 升级到 MaaFramework v5.13.0,并增强 Linux 控制器支持、配置兼容性、验证以及运行时选项恢复功能。
新功能:
错误修复:
增强功能:
构建:
CI:
文档:
测试:
Original summary in English
Sourcery 摘要
将 MaaPiCli 升级至 MaaFramework v5.13.0,并增强 Linux 控制器支持、配置兼容性、验证机制和运行时选项恢复功能。
新功能:
错误修复:
增强功能:
构建:
CI:
文档:
测试:
Original summary in English
Summary by Sourcery
Upgrade MaaPiCli to MaaFramework v5.13.0 and strengthen Linux controller support, configuration compatibility, validation, and runtime option recovery.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation:
Tests: