fix: harden decision-scope boundaries and CI isolation - #4259
Conversation
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
|
Self-review complete on exact head Changed surfaces:
Validation:
Premerge selected 18 checks; 17 passed. The sole failure is the existing Future-facing pass: applied only the bounded scenario-ownership cleanup (lifecycle/subagent assertions moved to typed-actions); broader framework work was intentionally not added. |
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
审阅对象:PR #4259,exact head 0a9d7635f1897e6606f545bff926a6680ee2e28c,合并提交 84334f794126735a83a6cfb77882dd58ca8066d9(base 5b163c37c)。本记录是该 exact head 的合并后审计,覆盖该 head 的完整增量(10 files, +3722/-3201)。
动机
这个 PR 同时修了三件互相牵连、但各自都成立的问题:
- decision scope 边界的运行时校验缺失。上一轮我审 #4257 时给出 APPROVE,同时在残余风险里点名了它的固有边界:
typing.cast只改注解、不做运行时检查,一旦已知 operation 的返回形状漂移,mypy 不会再报错。这个 PR 正是那个缺口的收尾——把 cast 换成按 operation 区分的运行时 schema 校验,并 fail closed。 - CI 失败归因被混在同一个必需检查里。
checks之前把内核静态检查(ruff/mypy/TS typecheck/control-plane 覆盖率/CLI 预算 smoke)与 dashboard 覆盖率 + 浏览器验收塞进同一个 job,导致某一侧的失败会掩盖另一侧的可见性——这也正是先前 #4246、#4244 连续两轮把 reload fixture 缺陷误读成"那两个 PR 引入"的结构性原因之一。 - 浏览器验收是单个 3.2k 行脚本。它没有场景隔离,criteria 编号 1-24 里 20 被用了两次,出现失败时只能靠读一大段日志定位。
三点合起来是一个完整可用的增量:边界更安全、失败可归因、验收可定位,而不是三份互不相干的清理。
改动思路
decision scope:规则 owner 仍然是 TypeScript(evaluateDecisionScope),Python 只在自己的编解码边界做形状校验。_evaluate 从返回 Any 改为返回 object,并要求结果信封(todo_decision_scope_result_v0 且含 result);新增一个共享的 _projection 做“必须是 dict + schema_version 命中该 operation 的允许集合”,_required_projection / _optional_relation 分别表达“不可空”和“可为 None”,_relation_matrix 额外校验矩阵必须恰好是 len(gates) × len(items),decision_scope_covers 要求真正的 bool。方案复用了 owner 已经发出的 schema 词表(standing_decision_authority_v0、required_decision_scope_consistency_v0、decision_scope_relation_v0、todo_gate_relation_v0),没有在 Python 侧另立一套规则;顺手把 _authority(authority) 的重复调用收敛为一次。
CI:把原 checks 拆成 kernel-static-checks(保留原有静态检查步骤)与 dashboard-acceptance(test:dashboard:coverage + 浏览器 smoke),checks 变成一个 2 分钟的聚合 job:needs: [changes, kernel-static-checks, dashboard-acceptance]、if: always() && needs.changes.outputs.core_tests == 'true',并执行 test "$KERNEL_RESULT" = success / test "$DASHBOARD_RESULT" = success。关键是保留必需检查名 checks 与其下游(pytest、merge-gate 仍依赖 checks),因此分支保护语义不变、dashboard 仍在合并路径内。
验收拆分:抽出 examples/personal-workspace-browser/fixture.mjs(共享启动/API mock)与 scenario-context.mjs,把断言原样搬进 navigation-sorting.mjs、chat-recovery.mjs、typed-actions.mjs,入口脚本按 scenarioCatalog 顺序执行并逐场景报告状态/note/耗时,新增可选的 LOOPX_PERSONAL_WORKSPACE_SCENARIO 选择器(未知 id 在起服务前就报错)。
具体改动
运行时代码:loopx/control_plane/todos/decision_scope.py(~+180/-60,删掉 cast,新增四个校验 helper 与四个 schema 常量)。CI:.github/workflows/python-tests.yml(77 行改动)。验收:examples/personal-workspace-browser-smoke.mjs(-3150,仅剩入口/调度)与新增 personal-workspace-browser/{fixture.mjs 1264、typed-actions.mjs 1392、navigation-sorting.mjs 347、chat-recovery.mjs 197、scenario-context.mjs 54}。测试:新增 tests/control_plane/test_todo_decision_scope_runtime_boundary.py(+140)与 tests/test_python_ci_workflow.py(+55/-4)。
关键代码讲解
_projection/_required_projection/_optional_relation(decision_scope.py):校验的唯一落点。非 dict、含非字符串键、schema_version不在允许集合时抛invalid typed decision scope <operation> …;nullable=True才允许显式None。因为按集合成员判断而不是相等判断,未来新增版本必须显式登记,符合“fail closed、不猜”的方向。_relation_matrix:把relations的返回从"cast 成嵌套数组"变成"必须恰好是len(gates)行 ×len(items)列的矩阵,且每个元素要么是 None 要么是合法 relation"。测试覆盖了{}、[]、[[]]、[[None],[None]]、[[{unknown_schema}]]五种畸形形态。decision_scope_covers:isinstance(result, bool)之后才返回;空 scope 的短路仍然在求值之前,语义与开销不变。.github/workflows/python-tests.yml的checks聚合:if: always()保证依赖失败/取消时聚合 job 仍会运行并给出失败结论;needs.*.result的字符串比较确保failure/cancelled/skipped都不能通过。tests/test_python_ci_workflow.py不只是正则匹配 YAML,而是真正执行这段 shell,对四种 lane 结果的 16 种组合逐一断言。- 入口脚本的
scenarioCatalog与LOOPX_PERSONAL_WORKSPACE_SCENARIO:默认跑全部场景(CI 覆盖不变),未知 id 立即报错;每个场景独立 try/catch,失败时记录该场景 id 与捕获到的错误并返回非零退出码。
对主干的风险
运行时校验:这是热路径上的新增检查,因此需要确认它不拒绝合法数据。证据是该 head 上 99 项本地测试通过,其中既包含三个 decision-scope 既有套件(走真实 TypeScript owner 端到端),也包含新的边界套件(走注入的畸形结果);合法值原样返回,唯一的行为差异是畸形值现在抛 typed TypeError 而不是被当作合法值使用。代价是有意的:若 owner 未来新增一个 relation schema 版本而 Python 侧未同步,调用会 fail closed——这正是这个 PR 想要的方向,且 schema 集合是命名常量、有测试兜底。
CI 拆分:风险在于"拆了会不会悄悄放宽门禁"。这里有三重证据:必需检查名 checks 未变;pytest/merge-gate 仍以 checks 为依赖(dashboard lane 仍在合并路径内);聚合脚本在 always() 下运行并对 success|failure|cancelled|skipped 的 16 种组合逐一双向验证(测试直接执行 shell)。core_tests != true 时跳过 checks 的行为与拆分前一致。
验收拆分:风险在于"搬家时掉了断言"。我按 pass(<n>, …) note 集合做了 base/head 对照:base 有 24 个不同 note(其中编号 20 被两个不同 note 复用),head 有 24 个 note;唯一不再单独出现的 note 是 base 里重复编号的 "Empty and populated Tasks boards keep identical width and four equal columns at desktop and wide desktop viewports",而对应的断言仍存在于 typed-actions.mjs(约 620-671 行的 assertBoardGeometry/宽度差值检查),失败仍会让场景报错。因此覆盖未丢失,但criterion 编号重复被消除后,这条验收不再有独立标签——这是本轮唯一的建议项(非阻断):建议后续给这组看板几何断言补一个独立 note/编号,使其在验收报告里保持可见。作为参考,该 head 的 dashboard-acceptance lane 全绿,26/26 检查通过。
补充说明:PR 正文自述的 premerge 唯一失败项(heartbeat_prompt_json 接口预算 3825 > 3600)与本 PR 改动面无关,且已声明在 origin/main 上同样复现。
本地验证(该 exact head 的独立 worktree):pytest -q tests/control_plane/test_todo_decision_scope_runtime_boundary.py tests/control_plane/test_todo_decision_scope_consistency.py tests/control_plane/test_todo_decision_scope_lifecycle.py tests/control_plane/test_todo_decision_scope_cli_validation.py tests/test_python_ci_workflow.py -> 99 passed;python -m mypy -> Success: no issues found in 22 source files;ruff check(改动文件)-> All checks passed!。hosted:26/26 全绿,含 kernel-static-checks、dashboard-acceptance、聚合 checks、pytest、merge-gate。
我的整体评价
APPROVE。这是一个把"上一轮自己指出的残余风险"真正闭环的改动:decision scope 编解码边界从"cast 掉类型"变成"按 operation 校验 schema 与形状并 fail closed",而且校验词表来自 TypeScript owner 而不是新造规则;CI 在保留必需检查名与合并路径的前提下把两条工具链拆成可独立归因的并行 lane,并用真实执行 shell 的测试证明聚合不会漏判;浏览器验收按场景拆分、共享 fixture,断言与 criteria 数量对照后确认没有丢覆盖。
无阻断发现。唯一建议(非阻断):把 typed-actions 里的看板几何断言恢复成独立的 criterion note,避免它只作为其他步骤的一部分被报告;另外,未来新增 decision-scope schema 版本时需同步扩展 Python 侧命名集合(这是有意 fail closed 的代价,已有测试兜底)。残余不确定性:本轮没有在本地复跑浏览器场景(依赖该 head 上已通过的 dashboard-acceptance lane 与 criteria/断言对照)。
English verdict: APPROVE - #4259 at exact head 0a9d763 (merged as 84334f7) closes the residual limit flagged in this lane's #4257 audit by replacing unchecked typing.cast annotations in loopx/control_plane/todos/decision_scope.py with operation-specific runtime schema validation that fails closed (result envelope required; dict + declared schema-version membership per operation; a real boolean for covers; an exact len(gates) x len(items) matrix for relations; explicit null accepted only where nullable), reusing the TypeScript owner's emitted schema versions rather than inventing a Python rule. It also splits the required checks job into parallel kernel-static-checks and dashboard-acceptance lanes behind a 2-minute aggregate that keeps the required check name and the pytest/merge-gate dependency chain, and refactors the 3.2k-line browser acceptance into three isolated scenarios over a shared fixture with an optional scenario selector. Verified at the exact head: 99 local tests pass (boundary suite plus decision-scope suites plus the CI workflow suite, which executes the aggregate gate for all 16 lane-result combinations), mypy and ruff are clean, and hosted checks are 26/26 green including both new lanes; criteria comparison shows no acceptance coverage lost (24 distinct criteria notes in both revisions, with the base's duplicated criterion 20 removed while its board-geometry assertions still run inside typed-actions). No blocking finding; the one advisory is to restore a distinct criterion note for those board-geometry assertions, and future decision-scope schema versions must be registered in the Python sets by design.
Summary
checksaggregateValidation
cqr_39ac7933b51d2d94c175Premerge note
The risk-based premerge suite passed 17/18 checks. Its only failure, the existing
heartbeat_prompt_jsoninterface-budget overage (3825 > 3600), reproduces unchanged onorigin/main; this PR does not alter that surface or relax its budget.