Repository navigation
test(refresh): reuse parser defaults for goal channel cases - #5866
huangruiteng merged 1 commit into
Conversation
Signed-off-by: hyk <4408344+hhyykk@users.noreply.github.com>
hhyykk
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; ultra.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机
维护 refresh-state CLI 测试的贡献者在运行现有 CI 时会遇到此问题。这个测试模块包含 15 个 Goal Channel 用例;当前 main 上 13 个在到达自身断言前失败,只有 2 个通过。例如,strict JSON 测试传入 NaN 时,旧的手写 Namespace 因缺少 todo_id 在 JSON 校验前失败;新 helper 通过 parser 生成当前默认值后,同一用例到达 strict JSON 检查,并确认刷新调用为零。现有错误会让 CI 红灯且遮住这些测试原本要保护的行为。现有 15 个 Goal Channel 测试现在能运行到自身断言;本精确 head 全部通过,已有断言及生产代码保持不变。本 PR 不改变 refresh-state 行为、Goal Channel 投递策略、真实 Goal 状态或外部投递。
改动思路
测试入口 _args() 通过 build_cli_parser() 创建共享 CLI 解析器,再调用 register_refresh_state_command() 注册当前命令语法。参数解析由现有 CLI parser 和 refresh-state 注册器持有;_args() 只给这个测试场景显式提供 Goal、Agent、classification、JSON 输出和按例启用的 sink suppression。返回的 Namespace 直接交给现有 handle_project_lifecycle_command()。这把两个当前由 handler 直接读取的 Todo/Turn 默认值交给 parser 生成,避免测试另存一份参数清单。该 helper 不写状态,测试中的刷新与通知提供者仍是 mock。
具体改动
精确 head 8ca6b898d23935fee23d37dccbd680a16469a3ae 只改一个测试文件,新增 18 行、删除 34 行:新增共享 parser 与格式 helper 导入;以 refresh-state --goal-id ... --agent-id ... --classification validated --format json 解析参数;仅在 suppress_external_sinks=True 时追加该开关。test_refresh_state_applies_goal_channel_delivery_postcondition 保留成功和失败结果断言;strict JSON 的 NaN/Infinity 用例仍要求刷新未调用;异常 redaction 用例仍要求隐藏私有路径与 token;sink suppression 用例仍要求禁止外部投递。测试断言没有改动。
本改动没有单独的 RFC、issue 或测试 fixture 规范。适用的现有验收是当前 refresh-state parser 与其既有 Goal Channel 断言;我对照了 loopx/cli_runtime.py 的 parser builder、refresh-state 命令注册器、docs/integration.md 的 refresh-state 描述,以及 docs/state-interaction-model.md 对外部 sink 抑制的契约。没有新增或更改状态词、持久格式或用户可见协议。
正向路径:测试调用 _args(suppress_external_sinks=True),共享 parser 产生完整 Namespace,处理器收到 suppression 选项,mock Goal Channel syncer 观察到 external_sink_delivery_authorized=False。负向路径:NaN 用例把非法 usage JSON 加到相同 parser 生成的 Namespace;处理器在调用刷新前拒绝输入,测试继续断言错误包含 strict JSON 且刷新调用列表为空。另有 mock sink 异常用例验证脱敏结果。所有这些是 CLI 测试边界,不是真实外部投递验证。
对主干的风险
最大的风险是未来 refresh-state 参数变化后,测试又在目标断言前因手写 Namespace 过期而失败。本 diff 删除了这份副本并复用生产 parser;若必填 CLI 参数变化,解析阶段会明确失败,提醒维护者同步测试场景。影响范围局限于这个测试 helper,不影响运行时。精确模块 15 项通过,邻近 Explore Graph 与外部投递模块 8 项通过;Ruff 与 git diff --check origin/main...HEAD 通过。canary 的 4 项直接检查通过,目录未选出额外 catalog 检查;候选路径的公共边界扫描无私有本地状态或凭证,未报告失败、跳过或人工 hold。远端 DCO、dependency-review 与 Python Tests workflow 在读取时仍排队;本地证据覆盖本 diff 的参数构造和断言,但完整远端工作流仍是合并准备门禁。测试通过 monkeypatch 提供刷新、通知和输出边界,没有验证真实 Goal 文件写入或 provider 投递。
同作者近期 PR #5861 修正停止/归档 Goal 准入断言;#5863 使用现有软认领策略恢复 Workspace 演示。这些分别处理协作准入、演示 Goal 配置和当前 refresh-state 测试参数 fixture,没有重复的 smoke、helper 或同一断言。本改动压缩现有夹具并修复独立的 13 项失败,没有新增重复 smoke。
我的整体评价
没有阻断发现。这个小型测试维护变更删除重复的参数默认值知识,并让 15 个现有安全与错误处理断言重新可执行,范围与代价相称。持续维护收益得到 current main/head 对照和精确模块运行支持;用户体验与运行时语义保持不变。剩余限制是测试仍 mock 真实 Goal 和外部 sink,且远端 CI 尚未完成。建议 APPROVE;合并准备仍须等待远端检查读回。
English verdict: APPROVE at 8ca6b89. The test-only change replaces a stale handcrafted Namespace with the current refresh-state parser; all 15 focused tests, 8 adjacent tests, Ruff, and diff checks passed. DCO, dependency review, and Python Tests are still pending, so merge readiness remains separate.
loopx-agent
left a comment
There was a problem hiding this comment.
Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; xhigh.
动机
建议 APPROVE,核验版本为 8ca6b898d23935fee23d37dccbd680a16469a3ae,对照 base 0475fce1a546af0f26206f71b4d04f8ac3169a4d。
维护 refresh-state 与 Goal Channel 的贡献者运行既有测试时,会遇到夹具先于安全断言失败的问题。例如传入 NaN 的用例,base 因手写 Namespace 缺少 todo_id 返回参数异常,尚未触及严格 JSON 检查;head 通过生产 parser 补齐默认字段后,同一用例明确拒绝 NaN,并保持刷新调用为零。原有安全、失败处理与幂等恢复检查重新到达自己的断言,而不是只消除测试红灯。交付范围是测试夹具维护;运行时、外部通知策略和真实用户操作保持既有行为。
改动思路
_args() 复用 build_cli_parser() 的全局语法及 register_refresh_state_command() 的命令注册,显式传入 fixture Goal、Agent、classification 和 JSON 格式,仅在对应场景追加 suppression 开关。解析所得 Namespace 交给既有 handle_project_lifecycle_command(),继续使用原来的刷新、通知与 post-writeback 提供者边界。parser 是参数默认值的唯一来源;本改动没有创建第二份执行或状态规则。
本测试夹具构造没有独立已接受的 RFC/规格。评审先读了上述精确 base 的 docs/integration.md State Refresh 和 docs/state-interaction-model.md State Refresh / suppression 恢复契约,再对照原有测试要求。相关产品契约保持;这里不把 fixture 修复当成全部外部投递或 Goal 恢复验收。
具体改动
完整 diff 只有 tests/cli_commands/test_project_lifecycle_goal_channel.py,+18/-34:增加两个 parser helper 导入;删除手写 33 字段的 Namespace;通过真实注册器解析固定 argv。所有七个 test_* 函数及其参数化 decorators 的 AST 与 base 完全相同,15 个本模块实例的断言没有增删或放宽。对 suppression=False/True 各自比较,33 个旧字段值全部相等;新 Namespace 有 56 个字段,Todo、Turn、replan 默认保持 None。
正向路径:默认 fixture 进入 handler,成功的通知 postcondition 仍得到成功返回;suppression=True 进入既有通知同步器时,external_sink_delivery_authorized 仍为 False。负向路径:NaN/Infinity/-Infinity 在刷新前被严格 JSON 拒绝;六种通知异常仍脱敏并保留分类后的错误原因;关闭 post-writeback hook 时 projection 调用为零。原有真实隔离 CLI 场景继续验证通知失败、同原身份重放、一次 spend,以及 sidecar producer 不重复执行。通知提供者和若干刷新路径是 mock;没有发出真实外部消息。
对主干的风险
最主要风险是把 parser 与 fixture 绑定后,未来默认变化被测试被动采用。现有关键行为由保留的独立断言约束,固定 Goal/Agent/classification/输出和 suppression 输入仍显式声明;本次旧字段逐项相等,且 production diff 为空。它恢复的是现有检查的可达性,没有改变默认授权、状态词表、领域规则或机器义务。
独立验证使用同一解释器与依赖,分别加载不可变 base/head 源码:相同三模块命令在 base 得到 13 failed / 10 passed,在 head 得到 23 passed,零失败或跳过。其中本模块 15 项,邻近 Explore Graph 与 external-delivery 8 项。base 的严格 JSON 反例明确显示缺少 todo_id,handler 在相同早期 scope 构造中还读取 turn_instance_id;head 的完整检查覆盖原失败条件。
执行命令:python -m pytest -q tests/cli_commands/test_project_lifecycle_goal_channel.py tests/cli_commands/test_project_lifecycle_explore_graph.py tests/control_plane/test_refresh_external_delivery.py。Ruff 与 diff 检查通过;精确 base 的原生 canary premerge --from-git-diff --git-diff-base 0475fce1a546af0f26206f71b4d04f8ac3169a4d 执行 4 项直接检查,catalog 选中 0 项,零失败、跳过或人工 hold。独立全 diff 公共边界检查没有新增私有路径、凭据或日志。按当前能力配置没有读取或等待远端 CI;这些结果不代表完整远端工作流通过。
现有覆盖扫描包括 Goal Channel、post-writeback、refresh external-delivery 与 Explore Graph 的 owning modules;本 PR 修复已有 helper,未复制 smoke。也检查了同作者近期批次及相关文件:#5861 的 stopped-goal 准入、#5873 的 heartbeat/wait 读模型、#5849 的 Team Plan browser recovery 分属不同消费者,不是重复本次断言。#5863 的演示配置也不替代本模块的安全检查。
我的整体评价
没有阻断发现。长程维护收益正向:从原本在 fixture 缺字段处停住,变成实际约束严格输入、脱敏、关闭隔离与一次结算;未来新增参数默认值也由同一 parser 维护。运行时体验保持,未增加用户导航、参数录入或确认步骤。两个测试运行耗时相近,不能据此宣称运行速度提升、模型净成本下降或真实外部投递验收完成。
相关的未来维护简化已经应用:退掉 helper 内的重复参数默认值知识,复用现有注册器;没有需要在此 PR 扩展的生产 refactor 或新抽象。此切片单用途、可逆,建议 APPROVE。
English verdict: APPROVE at 8ca6b89. The test-only parser reuse restores existing safety and recovery assertions: the immutable-base comparison has 13 failures, while all 23 head cases pass with unchanged test ASTs and identical prior parameter values. Ruff and four direct canary checks pass; no runtime speedup or live delivery qualification is claimed.
The existing refresh-state Goal Channel test helper builds its own
Namespaceand lacks the current parser-provided Todo/Turn fields. Thirteen cases fail before reaching their intended strict-JSON, post-writeback, error-redaction or external-sink suppression paths.Build the fixture arguments through the existing CLI parser and refresh-state registration instead. Preserve the explicit fixture Goal, Agent, classification, JSON output and suppression opt-in. This removes the stale default-field copy while keeping every existing assertion and all production behavior unchanged.
Validation: all 15 existing module cases and 8 adjacent Explore Graph/external-delivery cases passed; Ruff and diff checks passed. Risk-based canary passed 4 direct checks and selected no catalog checks. Candidate path scan found no private local state or credentials. Tests use isolated fixtures and simulated sinks; no live Goal or external delivery was exercised.