fix(review): align behavior disclosure contract - #3940
huangruiteng merged 1 commit into
Conversation
Signed-off-by: song <liusongstep@gmail.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
这个 PR 修复 pull-request-review capability 的 machine-readable behavior-disclosure 规则与 required contract test 之间的一处精确措辞漂移。main@5c56d386 中规则写成 “Flag silent changes”,而 contract test 将 “silent behavior changes” 当作稳定语义锚点,因此所有基于该 main 的 required pytest 都会继承确定性失败。最近的风险扩展本意是同时覆盖 runtime 与自动加载 instruction surface,正确修复应恢复稳定短语而保留扩展后的范围;无需削弱测试或引入新机制。
改动思路
变更只触及 build_review_execution_contract() 返回的 behavior_change_disclosure.rule 文案:把 Flag silent changes that alter existing... 改为 Flag silent behavior changes that alter existing...。这样消费者继续得到同一条 guidance-vs-obligation 规则,自动加载 skill/prompt/help 等 instruction surface 的风险范围没有退回,required test 的公共短语也重新对齐。
正向路径是 loopx pr-review 构造 execution contract → applicability 要求 behavior-change disclosure → agent 看到稳定短语并审计 runtime/自动加载指令面 → contract test 通过。负向反事实是若以后再次删改该短语,focused test 会立即失败;本 PR 没有改变 verdict、authority、typed state、activation 或默认执行逻辑。
具体改动
全 PR 只有 1 个 production 文件、+2/-1,没有 docs、schema、install bundle 或 generated artifact 变更。
关键代码讲解
build_review_execution_contract()是唯一 behavior-disclosure rule owner;本次只修改其返回字符串,不改变 evidence id、applicability 或 completion gate。behavior_change_disclosure.rule仍明确要求 runtime 或 automatically loaded instruction surfaces 的 default behavior changes 必须通过 renamed smokes、docs 或 release notes 披露。test_execution_contract_owns_deep_review_requirements继续作为稳定短语的回归门;修复实现而非放宽断言,保持对未来 silent-default drift 的检测能力。
对主干的风险
未发现 blocker。变更是对已存在 contract drift 的最小修复,不新增 activation surface,也不改变普通 Agent 行为的要求范围;因此 default-off isolation、authority semantics、typed-state rule 与 domain neutrality 均无新增风险。唯一可见差异是对 agent 的规则文字更明确,多了 behavior 一词,正是 required test 与审查语义想表达的内容。
我在独立 detached worktree 上验证:目标 exact head c03d3a5a653641cad67c53d97ee7d17d4f38c90b;focused pytest 6 passed(稳定短语与 bounded-wait 参数);Ruff 和 git diff --check 通过。远端 required pytest、build、Windows、dependency review、DCO 与 SonarCloud Code Analysis 全绿。全量 GitHub evidence 与本地结果一致。
我的整体评价
这是 scope 极小、职责清楚且能解除全仓 required suite 基线失败的修复。它保留原规则新增的 instruction-surface 覆盖范围,只恢复 machine-readable contract 已承诺的稳定措辞,改动/收益比例合理。对 exact head c03d3a5a653641cad67c53d97ee7d17d4f38c90b,结论为 APPROVE;未发现需要后续修复的代码级问题。
English verdict: APPROVE exact head c03d3a5a653641cad67c53d97ee7d17d4f38c90b. This one-line semantic repair restores the required silent behavior changes phrase without narrowing the expanded instruction-surface review scope. Focused tests pass 6/6, Ruff and diff check pass, and all required GitHub checks are green.
Summary
silent behavior changes;Why
Official
main@5c56d3868deterministically failstest_execution_contract_owns_deep_review_requirements: the test requiressilent behavior changes, while the contract currently sayssilent changes. This was surfaced by the full required suite on #3936 and reproduces on a clean latest-main worktree.The drift comes from
55dadee68("Harden instruction-surface review risk"), which reworded the rule toFlag silent changes that alter existing automation or ordinary agent behaviorwithout updating the test. Restoring the phrase in the rule text rather than loosening the test keepssilent behavior changesas the stable contract phrase the test guards, while preserving the broadened scope introduced by that commit.Because required
pytestruns on the merge commit withmain, every open PR currently inherits this failure (#3935, #3936, #3942 at the time of writing); they will re-run once this lands.The separate 1 ms host-loop timing failure from that run does not reproduce: all five bounded-wait parameters pass on the same clean snapshot.
Validation
pytest -q tests/capabilities/test_pr_review_contract.py tests/test_pr_review_github_scan.py- 15 passedpytest -q tests/test_host_loop_runtime_parity.py -k shell_worker_and_pi_share_bounded_wait_plan- 5 passedloopx canary premerge --from-git-diff --git-diff-base refs/remotes/official/main- 1/1 passedgit diff --check- passedFuture-facing scope pass
No companion refactor is needed: the implementation and test already share the correct owner; this only repairs their exact public contract wording.