Conversation
Signed-off-by: luw2007 <luw2007@gmail.com>
steven-kid
left a comment
There was a problem hiding this comment.
详细中文评审
结论:REQUEST_CHANGES。Exact head:f640844a54fdb80e7ca489d723b5a9dfcf020de0;base:9e2b6d425fff9b8b2ebd69a79c3d8491b12eb7b3。
需要修复两项已复现问题:P1:嵌套 plan 未经过公开字段约束,合成私有字段会直接出现在 CLI 输出;P2:数组类型的 status 导致未捕获 TypeError,破坏结构化失败协议。 以下意见基于真实子进程/CLI 反例,不包含真实私有数据。
动机
#3800 的目标是让独立配置装配系统提供可解释的配置事实,同时不复制 LoopX 的 Goal/Todo/quota/recovery 权威。维护者已明确把第一步收敛为 RFC、默认关闭的 probe/plan、合成 provider 和严格失败边界;因此本轮不要求补齐 apply、launch、真实 configs 集成或完整 extension 产品化,也不因只有合成 provider 就否定这一步。
不建立边界会让后续接入自行解释配置成功与任务完成;更小的合理方案是复用现有进程执行设施,只增加有明确数据形状的只读协议。当前 PR 的总体范围符合这个已接受切片,但“public-safe readback”和“严格 shape failure”是首个切片本身的必要条件,不能依赖未来消费者补救。
改动思路
新增模块通过 invoke_configuration_provider → run_capped_process 发送 JSON stdin,检查退出状态、响应 JSON、schema/operation/id,再返回受限 envelope。enabled=False 在进程调用前直接返回 disabled;通过 CLI --enable 才执行 provider。plan 的 digest 来自规范化 JSON,能够检查响应中 plan 与 digest 是否一致,但 digest 不证明内容公开安全、实际执行或任何授权。
我在 exact base/head 搜索了该符号及现有 run_capped_process 调用,阅读了 extensions/runtime.py 的 managed runner 与 docs/reference/extensions.md。新实现复用了现有 timeout/output-bound transport;它没有接入 managed extension manifest/lifecycle,而是提供直接的模块 CLI。对于维护者接受的合成契约切片,这是可解释的边界,不应描述成已经具备完整安装、注册和授权管理的扩展。
正向实测:合成可执行文件接收真实 stdin,回显合法身份和 digest,CLI 返回 available=true、ready 与 plan;unknown 保持 unknown,没有转换成任务完成。负向实测:超大 stdout 返回 response_too_large,关闭状态返回 disabled。另两个负例则暴露了下述公开输出和形状校验问题。
具体改动
关键代码讲解
configuration_assembly_provider.py::invoke_configuration_provider是新增 122 行模块的核心。调用前限制 operation/default-off,调用后检查传输失败与 envelope;然而对 status 的集合查询早于类型判断,对 plan 仅要求 dict 后整包回传。plan_digest使用排序键和紧凑 JSON 编码计算 SHA-256;它适合一致性核对,不能替代嵌套字段/schema 的公开边界。当前plan_id也只检查为字符串,后续应与定义好的标识符契约一起约束。main提供 argparse 的 probe/plan、operation-id、provider、enable、timeout 参数,调用函数并打印 JSON。其退出码表示 available,而不是 validated completion;因此保留 unknown 且 available=true 不能解读为业务成功。tests/test_configuration_assembly_provider.py新增 97 行、四项测试,涵盖关闭态、合法 plan/incomplete、顶层 private 丢弃、schema/id/digest 不匹配及若干进程失败。合成脚本走真实进程传输,但合法 digest 由被测模块的plan_digest生成;本轮额外 fixture 独立计算 digest,并覆盖嵌套字段和非字符串 status。
两份各 44 行的中英文 RFC 内容对应,声明 Draft/Partial、可选 extension 边界、只读 v0、64 KiB/timeout、Unknown 保留及无任务完成权限。它们将“仅白名单字段进入 readback”列为协议承诺,也正是需要代码落实的地方。全部四个文件已阅读;没有生成产物或运行时持久状态迁移。
对主干的风险
[P1] plan 整包穿透公开 readback。 位置:loopx/configuration_assembly_provider.py:94。provider 返回 plan={"revision":"public-v1","private_note":"SYNTHETIC_PRIVATE_SENTINEL"},独立计算匹配 digest,其余字段完全合法。通过实际 python -m loopx.configuration_assembly_provider plan ... --enable --provider <fixture> 调用,CLI 退出 0,stdout 的 plan 原样包含该标记。现有测试只在 envelope 顶层放置 private 字段,无法检查 plan 内部。
最小修复:为公开 plan 定义明确、版本化且受限的数据形状,检查允许字段与值的范围;未知/私有内容应拒绝或转换成明确的公开投影,不能因为 hash 匹配就直接返回原始配置内容。确需保留的原始 plan 应留在其私有 owner,以公开安全引用跨边界。回归需通过真实 CLI/子进程,覆盖嵌套未知字段、敏感内容占位符与合法 plan。
[P2] 非字符串 status 逃出结构化失败通道。 位置:同文件 :78。将合法响应的 status 改为 ["ready"],JSON 解析成功,但 status not in ALLOWED_STATUSES 抛出 TypeError。实测 stdout 为空、stderr 为 traceback;上游无法得到约定的 unknown/unavailable/failure_kind。应先检查字符串类型,再检查枚举成员;增加 list、dict、null 和非法字符串输入的进程级回归,统一返回 invalid_status。
独立验证:新增模块四项测试通过,既有 capped-process 两项测试通过,diff check 通过;额外六类真实 CLI 输入覆盖 valid、unknown、disabled、oversized、nested-private、invalid-status。未运行真实第三方 provider、全平台进程树验证和仓库全部 canary。本轮源数据没有 status-check rollup,未将缺失信息解释成 CI 成功或失败。
我的整体评价
这个首个切片有维护者认可的具体用途,复用 transport、限制 operation、保留 Unknown 的方向合理;不需要通过扩大功能来修复本轮问题。需要的是一个真正受限的公开 plan 契约,以及严格的错误类型入口。
当前默认关闭路径确实不会进入 provider 调用,新代码也没有任何 Goal/Todo 完成写入;但这些事实不能证明启用后的数据边界安全。嵌套原始数据穿透和非字符串 status 异常均已通过真实 CLI 复现,所以本轮请求修改。重新评审时应继续以完整 probe/plan 契约为单位,保留未执行的完整 baseline/provider/平台验证为 unverified,不把四项现有测试通过当作整体准入证明。
English verdict: REQUEST_CHANGES at exact head f640844. The maintainer-scoped synthetic read-only slice is justified, but a matching digest allows arbitrary nested plan fields into the public CLI result (P1), and status=["ready"] raises an uncaught TypeError instead of a structured failure (P2). Both were reproduced through the real module CLI and subprocess boundary using synthetic data. Define a bounded public plan schema and type-check status before enum membership. Four provider tests, two capped-process tests and diff check passed; six additional CLI cases exercised positive/negative behavior. Full provider/platform and baseline coverage remains unverified; no merge authorization.
|
English verdict: REQUEST_CHANGES at exact head f640844. The maintainer-scoped synthetic read-only slice is justified, but a matching digest allows arbitrary nested plan fields into the public CLI result (P1), and status=["ready"] raises an uncaught TypeError instead of a structured failure (P2). Both were reproduced through the real module CLI and subprocess boundary using synthetic data. Define a bounded public plan schema and type-check status before enum membership. Four provider tests, two capped-process tests and diff check passed; six additional CLI cases exercised positive/negative behavior. Full provider/platform and baseline coverage remains unverified; no merge authorization. |
Summary
probe/planprocess contract with strict schema, identity, digest, output-size, and timeout fencesunknown/incomplete, filter output to public allowlisted fields, and add a direct CLI fallback plus synthetic executable testsCloses #3800
Validation
python -m pytest -q tests/test_configuration_assembly_provider.py— 4 passedpython -m compileall -q loopx/configuration_assembly_provider.py— passedloopx canary premerge --from-git-diff— passedBoundaries
Default-off and read-only. Provider output cannot complete a task or grant gate, launch, confirmation, recovery, or writeback authority.
apply,observe,recover, real client launch, and Skills/workflow-kit integration remain excluded.