feat(pi): 支持 resource.hash 校验 - #3
Merged
Merged
Conversation
在 resource.path 加载后、附加资源加载前校验主资源 hash,不匹配时记录警告。
Contributor
There was a problem hiding this comment.
嘿——我发现了 2 个问题
面向 AI Agent 的提示
请处理本次代码审查中的评论:
## 单独的评论
### 评论 1
<location path="Impl/Runner.cpp" line_range="235" />
<code_context>
LogInfo << "Start Agent" << VAR(agent_param.child_exec) << VAR(os_args) << VAR(agent_param.cwd);
- auto& agent_child = agent_children.emplace_back(agent_param.child_exec, os_args, boost::process::start_dir = agent_param.cwd);
+ auto& agent_child =
+ agent_children.emplace_back(agent_param.child_exec.string(), os_args, boost::process::v1::start_dir = agent_param.cwd.string());
if (!agent_child.valid()) {
LogError << "Failed to start agent process" << VAR(agent_param.child_exec) << VAR(args) << VAR(agent_param.cwd);
</code_context>
<issue_to_address>
**issue (broader_impact):** 在将 `agent_param.child_exec` 和 `agent_param.cwd` 传递给 Boost.Process 之前将其转换为 `std::string`,会丢失 Windows 上原生的宽路径处理能力,因此可执行文件路径或工作目录包含非 ASCII 字符的 Agent 将无法启动,或会使用错误的路径。
**触发条件:** 在 Windows 上,当 Agent 的可执行文件路径或工作目录包含无法用当前窄字符编码表示的字符时。
**建议修复:** 将原始的 `std::filesystem::path` 值传递给 Boost.Process v1 构造函数和 `start_dir` 选项,或者在 Windows 上使用平台原生的宽字符串表示形式。
</issue_to_address>
### 评论 2
<location path="Impl/Configurator.cpp" line_range="174" />
<code_context>
return std::nullopt;
}
+ runtime.primary_resource_count = runtime.resource_path.size();
+ runtime.resource_hash = resource_iter->hash;
// Find current controller for attach_resource_path
</code_context>
<issue_to_address>
**issue (bug_risk):** 运行时现在实现了 PI v2.6.0 的资源哈希处理,但发送给 Agent 子进程的现有 `PI_INTERFACE_VERSION` 环境变量仍为 `v2.5.0`,因此 Agent 会收到过时的协议版本,并可能错误地禁用或误解 v2.6.0 的行为。
**触发条件:** 配置的 Agent 检查 `PI_INTERFACE_VERSION` 以确定客户端支持哪些 PI 功能时。
**建议修复:** 将对外公布的 `PI_INTERFACE_VERSION` 值更新为 `v2.6.0`,同时启用新支持的资源哈希功能。
</issue_to_address>Original comment in English
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="Impl/Runner.cpp" line_range="235" />
<code_context>
LogInfo << "Start Agent" << VAR(agent_param.child_exec) << VAR(os_args) << VAR(agent_param.cwd);
- auto& agent_child = agent_children.emplace_back(agent_param.child_exec, os_args, boost::process::start_dir = agent_param.cwd);
+ auto& agent_child =
+ agent_children.emplace_back(agent_param.child_exec.string(), os_args, boost::process::v1::start_dir = agent_param.cwd.string());
if (!agent_child.valid()) {
LogError << "Failed to start agent process" << VAR(agent_param.child_exec) << VAR(args) << VAR(agent_param.cwd);
</code_context>
<issue_to_address>
**issue (broader_impact):** Converting `agent_param.child_exec` and `agent_param.cwd` to `std::string` before passing them to Boost.Process loses the native wide-path handling on Windows, so agents whose executable or working directory contains non-ASCII characters fail to start or use the wrong path.
**Triggers:** On Windows when an agent executable path or working directory contains characters not representable in the active narrow encoding.
**Suggested fix:** Pass the original `std::filesystem::path` values to the Boost.Process v1 constructor and `start_dir` option, or use the platform-native wide-string representation on Windows.
</issue_to_address>
### Comment 2
<location path="Impl/Configurator.cpp" line_range="174" />
<code_context>
return std::nullopt;
}
+ runtime.primary_resource_count = runtime.resource_path.size();
+ runtime.resource_hash = resource_iter->hash;
// Find current controller for attach_resource_path
</code_context>
<issue_to_address>
**issue (bug_risk):** The runtime now implements PI v2.6.0 resource-hash handling, but the existing `PI_INTERFACE_VERSION` environment variable sent to Agent subprocesses remains `v2.5.0`, so agents receive a stale protocol version and can incorrectly disable or misinterpret v2.6.0 behavior.
**Triggers:** When a configured Agent inspects `PI_INTERFACE_VERSION` to determine which PI features the client supports.
**Suggested fix:** Update the advertised `PI_INTERFACE_VERSION` value to `v2.6.0` along with the newly supported resource hash feature.
</issue_to_address>保留 Agent 路径的原生编码,同步 PI_INTERFACE_VERSION 到 v2.6.0。单独查找 boost_process,避免顶层 Boost 配置因版本不匹配失败。
移除未使用的 BoostIO 头,避免引入 Boost.Process v2 链接符号,并改用现有 Boost 组件提供的头文件查找。
移除 Boost.Process 依赖,分别使用 CreateProcessW 与 POSIX 进程接口,保持原生路径编码并统一管理子进程生命周期。
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
Tests
Sourcery 总结
支持 PI v2.6.0 资源完整性验证,同时更新 Agent 进程处理方式和 Boost 依赖项。
新功能:
resource.hash字段;不匹配时发出警告,但不会阻止启动。改进:
构建:
文档:
测试:
resource.hash字段的解析测试覆盖。Original summary in English
Sourcery 摘要
在更新 Agent 进程处理方式和 Boost 依赖项的同时,新增 PI v2.6.0 资源完整性验证。
新功能:
resource.hash字段;出现不匹配时发出警告,但不会阻止启动。增强:
构建:
文档:
测试:
resource.hash字段的解析测试覆盖。Original summary in English
Sourcery 摘要
支持 PI v2.6.0 资源完整性验证,同时更新 Agent 进程处理方式及其 Boost 依赖项。
新功能:
resource.hash字段;出现不匹配时发出警告,但不会阻止启动。增强功能:
构建:
文档:
测试:
resource.hash字段添加解析器测试覆盖。Original summary in English
Sourcery 摘要
在现代化 Agent 子进程处理的同时,支持 PI v2.6.0 资源完整性验证。
新功能:
增强:
构建:
文档:
测试:
resource.hash字段的解析测试覆盖。Original summary in English
Sourcery 摘要
支持 PI v2.6.0 资源完整性检查,同时实现 Agent 子进程处理的现代化。
新功能:
增强:
构建:
文档:
测试:
resource.hash字段的解析测试覆盖。Original summary in English
Summary by Sourcery
Support PI v2.6.0 resource integrity checks while modernizing Agent subprocess handling.
New Features:
Enhancements:
Build:
Documentation:
Tests: