Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嗨——我已经审阅了你的改动，一切看起来都很棒！
帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据你的反馈改进后续评审。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
该 PR 为 MaaPiCli（ProjectInterface CLI Client）补齐对 PI v2.5.0 的协议支持，涵盖 v2.3.0～v2.5.0 的多项新增字段与行为（多级 option、checkbox、preset、group、以及 Agent 子进程 PI_* 环境变量）。
Changes:
- 扩展 PI 数据结构：新增 controller/resource 级 option、task.group、option.checkbox/default_case 变体、preset/group/global_option 等字段，并同步扩展配置结构以支持多级 option 与 checkbox 值。
- CLI 交互增强：展示/编辑全局、资源、控制器级 options；支持按 group 展示任务；新增 preset 一键应用；支持 checkbox 选项交互。
- 运行时增强：生成并传递 PI_* 环境变量给 Agent 子进程；解析 import 时合并 preset；合并 option override 按优先级生效。
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| source/include/ProjectInterface/Types.h | 扩展 PI/配置/运行时参数结构以支持 v2.3～v2.5 新字段（checkbox、group、preset、多级 option、env_vars 等） |
| source/include/ProjectInterface/Configurator.h | 新增 option 适用性判断与多级 option 合并的私有辅助函数声明 |
| source/MaaPiCli/README.md | 新增 MaaPiCli 说明与 PI 支持版本/CLI 局限文档 |
| source/MaaPiCli/Impl/Runner.cpp | 启动 Agent 子进程时注入 PI_* 环境变量 |
| source/MaaPiCli/Impl/Parser.cpp | import 合并 preset；配置校验中增加多级 option 列表的清理与 checkbox 校验 |
| source/MaaPiCli/Impl/Configurator.cpp | 生成 PI_* env vars；按优先级合并 global/resource/controller/task 四级 option overrides；支持 checkbox overrides |
| source/MaaPiCli/CLI/interactor.h | 增加 preset 应用与多级 option 处理接口声明 |
| source/MaaPiCli/CLI/interactor.cpp | CLI 展示/录入多级 option、checkbox；按 group 输出任务；支持 preset 应用 |
Comments suppressed due to low confidence (1)
source/MaaPiCli/CLI/interactor.cpp:1356
opt.default_caseis now avariant<string, vector<string>>, but the Select branch treats any non-empty variant as “has default” and then only reads the string alternative. Ifdefault_caseis (incorrectly) an array,has_defaultbecomes true whileconfig_opt.valuestays empty, which skips prompting and produces an invalid config entry. Consider making “has default” conditional onget_if<string>and also fall back to interactive selection when the default string is empty or not found inopt.cases.
auto has_default = std::visit([](const auto& v) { return !v.empty(); }, opt.default_case);
if (has_default) {
if (auto* str = std::get_if<std::string>(&opt.default_case)) {
config_opt.value = *str;
}
auto case_iter = std::ranges::find(opt.cases, config_opt.value, std::mem_fn(&InterfaceData::Option::Case::name));
if (case_iter != opt.cases.end()) {
selected_case = &(*case_iter);
}
}
source/MaaPiCli/CLI/interactor.cpp
Outdated
| auto has_default = std::visit([](const auto& v) { return !v.empty(); }, opt.default_case); | ||
| if (has_default) { | ||
| if (auto* vec = std::get_if<std::vector<std::string>>(&opt.default_case)) { | ||
| config_opt.values = *vec; | ||
| } |
There was a problem hiding this comment.
Checkbox default handling has the same variant issue: has_default becomes true for a non-empty string default_case, but the code only reads the vector alternative, so it will skip prompting and leave config_opt.values empty. Consider treating a default as present only when the variant holds a non-empty vector<string> and otherwise falling back to prompting (or logging an error).
| auto has_default = std::visit([](const auto& v) { return !v.empty(); }, opt.default_case); | |
| if (has_default) { | |
| if (auto* vec = std::get_if<std::vector<std::string>>(&opt.default_case)) { | |
| config_opt.values = *vec; | |
| } | |
| if (auto* vec = std::get_if<std::vector<std::string>>(&opt.default_case); vec && !vec->empty()) { | |
| config_opt.values = *vec; |
| for (int idx : indexes) { | ||
| config_opt.values.emplace_back(opt.cases[idx - 1].name); | ||
| } | ||
| } |
There was a problem hiding this comment.
For checkbox options, sub-options (case.option) are never processed: selected_case remains nullptr, so the nested sub-option recursion at the end of process_option() is skipped. Per PI spec, each selected checkbox case may have its own option list that should be applied; consider iterating over all selected cases (from defaults or user input) and calling process_option() for each case’s sub-options.
| } | |
| } | |
| // Ensure that at least one selected checkbox case is available for | |
| // sub-option processing later in process_option(). | |
| if (!config_opt.values.empty()) { | |
| for (const auto& value : config_opt.values) { | |
| auto it = std::find_if( | |
| opt.cases.begin(), opt.cases.end(), | |
| [&](const auto& case_item) { | |
| return case_item.name == value; | |
| }); | |
| if (it != opt.cases.end()) { | |
| selected_case = &(*it); | |
| break; | |
| } | |
| } | |
| } |
| auto check_option_list = [&](std::vector<Configuration::Option>& opts) { | ||
| for (auto it = opts.begin(); it != opts.end();) { | ||
| if (!data.option.contains(it->name)) { | ||
| LogWarn << "Option not found in config, removing" << VAR(it->name); | ||
| it = opts.erase(it); | ||
| erased = true; | ||
| } | ||
| else { | ||
| ++it; | ||
| } | ||
| } | ||
| }; | ||
| check_option_list(config.global_option); | ||
| check_option_list(config.resource_option); | ||
| check_option_list(config.controller_option); | ||
|
|
There was a problem hiding this comment.
Parser::check_configuration() now prunes unknown options from global_option/resource_option/controller_option, but it does not validate the option values against the option definition (e.g., select/switch value exists in cases, checkbox values all exist). This can leave an incompatible config that later silently does nothing at runtime. Consider reusing the same per-type validation logic as check_task() for these level-option lists (and prune invalid entries similarly).
source/MaaPiCli/Impl/Parser.cpp
Outdated
| auto check_option_list = [&](std::vector<Configuration::Option>& opts) { | ||
| for (auto it = opts.begin(); it != opts.end();) { | ||
| if (!data.option.contains(it->name)) { | ||
| LogWarn << "Option not found in config, removing" << VAR(it->name); |
There was a problem hiding this comment.
Log message wording is misleading: here you’re checking whether the option exists in the interface data (data.option.contains(...)), but the message says "Option not found in config". Consider changing it to something like "Option not found in interface, removing from config" to reflect what happened.
| LogWarn << "Option not found in config, removing" << VAR(it->name); | |
| LogWarn << "Option not found in interface, removing from config" << VAR(it->name); |
Summary by Sourcery
在 MaaPiCli 中增加对更新版本 ProjectInterface 协议的支持，包括全局/资源/控制器级选项、复选框和分组选项、预设，以及为代理提供的 PI_* 环境变量。
新功能：
增强：
Original summary in English
Summary by Sourcery
Add support in MaaPiCli for newer ProjectInterface protocol versions, including global/resource/controller options, checkbox and grouped options, presets, and PI_* environment variables for agents.
New Features:
Enhancements: