fix(todos): validate in task repository worktree - #4308
Conversation
0f79aa7 to
e8931a8
Compare
|
Request changes conclusion (author-owned PR; GitHub blocks formal self-review) 动机本评审针对 exact head 改动思路实现沿用了现有的 TypeScript completion transaction、Python 正向路径的边界是清楚的:A 的状态仍由 canonical Todo authority 写入,B 只提供校验执行环境;验证命令仍是 caller-approved argv,验证通过后才进入 Todo mutation/quota settlement。问题在于“clean”条件的时间点和异常归类还没有与真实 Turn 生命周期对齐,见下方阻塞项。 具体改动关键代码讲解
阻塞发现
对主干的风险本地 exact-head 验证通过了 completion-validation 与 lane-scope 共 51 个 Python 测试、Turn/Executor 共 121 个测试、Node transaction 6 个测试、Ruff、compileall 和 diff 检查;merge-tree 无冲突。新增的负例证明缺 receipt、外部仓库和预先脏 worktree 会 fail closed,但没有覆盖 host 写入后的生命周期,也没有覆盖 malformed receipt,因此不能证明主路径可用。 远端 exact head 的 required checks 也尚未满足: 我的整体评价PR 的责任边界和复用方向是正确的:TypeScript 继续拥有 effect/状态决策,Python 只做 workspace 适配,Goal repository 默认路径也保持兼容。当前 exact head 仍需 REQUEST_CHANGES:clean 检查发生在可能产生交付 diff 的 host 之后,且 malformed receipt 会越过 typed failure contract;此外远端 Stage2C required check 未绿。完成 baseline-aware cleanliness、统一异常到 typed receipt、补充真实跨仓库 Turn 正向/损坏 receipt 测试,并让 required checks 全部通过后,请以新的 exact head 重新提交 review。 English verdict: REQUEST_CHANGES for exact head |
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
English verdict: REQUEST_CHANGES — the task_repository-bound completion-validation seam is useful, but malformed delivery-workspace receipts escape as RuntimeError and the required remote Stage2C correctness gate is red.
动机
本次 review 针对精确 head e8931a87f65395d91e28f95fb7337ce47b12b1df(base main)进行。PR 的目标是把 Todo completion validation 绑定到 task_repository,并要求跨仓库验证使用 turn-bound、独立且干净的 delivery worktree。这解决了验证命令可能在错误仓库、canonical checkout 或 dirty checkout 中运行的问题;复用现有 completion effect、workspace receipt 和 Turn writeback 边界的方向是合理的。
不过这个新 authority input 还没有覆盖两类失败状态:typed delivery-workspace 解码失败,以及 merge-ref 上的 Stage2C crash-barrier 协议失败。前者会让 documented path-free validation_blocked_completion receipt 根本产生不出来,直接抛出 RuntimeError;后者是当前 required remote correctness gate 的真实红灯。因此本次结论为 REQUEST_CHANGES。
改动思路
主路径由 todo complete 和 managed Turn 的 validated-completion writeback 进入 completion_transaction effect,再由 Python completion_validation adapter 解析 task_repository 与 delivery_workspace。同仓库 Todo 保留 Goal repository 默认;跨仓库 Todo 则要求记录的 repository identity、独立 worktree、匹配的 workspace snapshot 和 clean git status,随后才执行 caller validation,并把通过 receipt 交给 completion transaction 提交。
这个 decision owner 选择是正确的:TypeScript 负责 effect shape/authorization,Python 负责工作区解析和进程执行,lifecycle 在失败时让 Todo 保持 open。但 _resolve_completion_validation_workspace 对共享 typed decoder 的异常没有映射到该状态机,导致一个非法 enum 进入 untyped exception path;而远端 merge-ref 的 stdout 顺序问题说明新旧 e2e harness 的 process barrier contract 仍未被可靠地守住。
具体改动
关键代码讲解
loopx/control_plane/todos/completion_validation.py:_workspace_failure(约 77-90 行)集中构造 path-free 的 blocked receipt,这是正确的失败投影 owner。loopx/control_plane/todos/completion_validation.py:_resolve_completion_validation_workspace(约 135-210 行)根据task_repository、workspace identity/kind、independent-worktree 要求和git status --porcelain=v1选择 Goal repo 或 recorded worktree;但在约 153-174 行直接调用normalize_delivery_workspace_snapshot,没有捕获 decoder rejection。loopx/cli_commands/turn.py(约 430-447 行)在存在 writeback causality 时捕获 managed Turn 的 delivery workspace,并把 selected Todo 的 repository 约束带入 completion effect。该捕获需要继续证明 snapshot 对应真实执行路径;当前专门的跨仓库 Turn e2e 被远端门阻断。loopx/cli_commands/todo.py、turn_todo_writeback.py、completion_transaction.py/.ts和loopx/todos.py把task_repository/validation effect 贯穿 CLI、Turn、transaction 和 Todo projection;这是一条 cohesive seam,而不是重复 runner。provider_terminal_lifecycle.py负责把验证结果接入生命周期;它能消费 typed blocked receipt,但无法处理当前逃逸的RuntimeError。docs/project-agent-todo-contract.md明确了 cross-repository、clean independent worktree 和 path-free receipt 契约;因此 decoder exception 不是文案差异,而是可观察行为违约。tests/control_plane/test_todo_completion_validation.py与tests/control_plane_ts/todo_completion_transaction.test.ts覆盖了有效、缺失、foreign、dirty 和 transaction paths,但没有覆盖 malformed typed snapshot,也没有锁定 merge-ref barrier 的 stdout contract。
对主干的风险
P1 — malformed delivery-workspace receipt 会以 RuntimeError 逃逸,绕过 completion state model。
触发:调用方提交 schema 版本正确但 typed enum 非法的 snapshot,例如 identity_kind="bad"(同时给出合法的 delivery_workspace_v1、repository 和 worktree 字段)。路径:_resolve_completion_validation_workspace → normalize_delivery_workspace_snapshot → TypeScript requireStringLiteral/decoder rejection。精确 head 的直接复现结果为 RuntimeError: identity_kind is unsupported;命令没有运行 caller validation,也没有返回文档承诺的 validation_blocked_completion/workspace_receipt_* path-free receipt。
这会把可恢复的输入/receipt 问题变成 managed completion 的异常中止,可能绕过“Todo 保持 open、等待修复”的 lifecycle 语义,并把内部 decoder 文本暴露到 CLI/Turn 边界。最小修复是:在 Python adapter 的 decoder boundary 捕获可预期的 typed rejection(或让 decoder 返回 Result),统一映射为已有的 workspace_receipt_invalid/validation_blocked_completion typed receipt;保证 validator 不执行、Todo 不提交,并保留可审计但不含路径的 reason code。加入 identity_kind、workspace_kind 等每个 literal 的 malformed negative test,以及 CLI/managed Turn 的 no-command-execution 断言。
P1 — required remote Stage2C correctness gate 在 merge-ref 上失败。
远端 stage2c (e2e 2) 和 stage2c-correctness-e2e 对精确 head 的失败集中在 tests/control_plane/test_shadow_drain_e2e.py::test_cursor_cannot_authorize_deletion_or_hide_mutations[string_seq]:crash harness 在 pending() 的 before_commit barrier 处收到以 { 开头的 JSON stdout,未看到预期 BARRIER 行,报 AssertionError: No process barrier: {;结果为 1 failed, 110 passed,随后 merge-gate 因 correctness check 非 success 而失败。当前本地精确 head focused test 可以通过,但这不能清除 required merge-ref 的红灯,反而提示合并 ref 的 subprocess stdout ordering/environment 仍未得到解释或修复。
最小修复是让 merge-ref 上的 subprocess 严格先发可识别的 barrier、再发 JSON/诊断输出,或修复 harness 对 stdout/stderr 的协议读取;同时补充该 string_seq 路径的 exact merge-ref regression coverage,并重新跑 stage2c、stage2c-correctness-e2e 和 merge-gate。未完成前不能把本 PR 视为可合并。
验证结果:tests/control_plane/test_todo_completion_validation.py 与同域 lane suite 共 51 passed;TypeScript completion transaction focused tests 6 passed;非法 identity_kind 的直接复现稳定得到上述 RuntimeError;本地 string_seq focused test 1 passed,但远端 required Stage2C 仍是失败状态。以上失败均针对 exact head,不是基于旧 head 或纯静态推测。
我的整体评价
把 completion validation 绑定到 task_repository 并引入 clean independent delivery workspace,是一个有价值且边界清晰的 control-plane 改动;同仓库默认路径和有效跨仓库路径也有正向覆盖。当前的主要问题在于 failure semantics 尚未闭合:共享 decoder 的异常没有被归一化到 typed receipt,required merge-ref barrier 也没有通过。请先补齐 malformed receipt 的 fail-closed、no-command-execution 回归,修复并回读两个 Stage2C correctness checks,再以新的 exact head 重跑完整 focused/negative/e2e 验证后重新申请 review。
e8931a8 to
c463871
Compare
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
c463871 to
69d1998
Compare
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机
本 PR 的目标是把 Todo 的 caller-approved completion validation 绑定到任务实际所属仓库。原实现只有 Goal repository/cwd 这一默认路径,跨仓库 Todo 无法证明命令将在正确、隔离且干净的 checkout 中执行;同时,delivery-workspace receipt 中的非法 typed literal 会让 TypeScript decoder 的 RuntimeError 直接冒泡,既没有稳定的 lifecycle 状态,也可能让调用方误以为是内部崩溃。受影响的是 todo complete CLI、managed Turn writeback 和共享 completion transaction。当前 head 保留无 task_repository 时的 Goal repo 默认行为,只对显式跨仓库选择增加 receipt/worktree 约束。
改动思路
事务决策仍由 completion_transaction.ts 持有:它判断是否需要 validation、构造唯一的 caller_validation effect,并在 effect 返回通过 receipt 后才允许 durable completion。Python completion_validation.py 只是 host-effect adapter,复用既有的 capture_delivery_workspace、normalize_delivery_workspace_snapshot、canonical repository identity 和 caller-validation receipt schema。跨仓库路径要求 Turn-bound receipt、independent_git_worktree、canonical origin/identity 匹配和 clean git status;缺失、非法、冲突、脏或不可验证都在执行命令前返回 path-free typed failure。修复将 decoder boundary rejection 映射为既有 receipt 状态 workspace_receipt_invalid,不吞掉为成功,也不引入第二套异常协议。CLI 与 managed Turn 传入同一 effect 参数,前端/Lark 没有新增配置源。
具体改动
关键代码讲解
loopx/control_plane/todos/completion_validation.py::_resolve_completion_validation_workspace:无task_repository时直接保持 Goal repo fast path;跨仓库时先验证 receipt,再验证当前 worktree 的 canonical identity、独立 worktree 类型和 cleanliness。新增的窄try/except (RuntimeError, TypeError, ValueError)只包围 receipt decoder,把非法输入转为workspace_receipt_invalid,并明确保持local_path_captured=false。loopx/control_plane/todos/completion_validation.py::run_declared_completion_validation_effect:继续拒绝非caller_validation或非法 argv,将task_repository、Turn receipt 和验证 workspace 交给现有 runner;它不拥有 Todo policy 或 commit 决策。loopx/control_plane/todos/completion_transaction.ts::reduceTodoCompletionTransaction:把 canonical Todo 的可选、非空task_repository加入 effect,保留现有两次 reduction/一次 external effect 顺序。loopx/cli_commands/todo.py与loopx/cli_commands/turn.py:分别从 settlement writeback 或当前 exact Turn 捕获 workspace receipt,再调用同一 completion writer;没有新增 cwd CLI option。
测试新增了真实跨仓库 clean linked worktree 的 positive path,以及 missing/foreign/dirty/malformed receipt 的 fail-closed paths。畸形 receipt 回归明确断言 validator 调用次数为零、Todo 仍为 open、结果为 validation_blocked_completion 且不含本地路径。
对主干的风险
旧 review 的两个 P1 已处理并在最终 head 复核:
- malformed receipt 不再冒泡裸
RuntimeError,而是稳定的workspace_receipt_invalidtyped receipt;新的负向测试证明命令不会执行。 - 原先远端
stage2c-correctness-e2e的string_seqbarrier 失败在修复 headc4638719a上重跑通过;rebase 到最新主干后的新 head69d19988b仍在等待对应 workflow 完成,未对 shadow harness 做无关修改。旧失败因此更像 merge-ref/跨进程时序问题,而不是本 PR 的回归;最终以新 head 的 required check readback 为准。
本地证据:completion/lane Python 52 passed;shadow cursor 参数 7 passed;control-plane TypeScript full suite 1449 passed、1 skipped;npm run typecheck:control-plane 通过;Node 22.18.0 transaction tests 6 passed;changed-file Ruff、py_compile 和 public-boundary scan 通过。完整 mypy 在仓库基线上仍有 964 个既有错误,未作为本次代码回归。premerge canary 已执行完整风险集:direct checks、公共边界和 risk-profile smokes 通过;quota-plan-smoke 的 effect-runtime internal error 可在同一 origin/main 基线复现,因此作为明确环境基线失败记录,不绕过该结果。
变更范围为 11 个文件(+519/-39),所有运行时代码都有 CLI/Turn active caller;task_repository 是可选字段,默认关闭的 Goal repo 路径保持不变。rebase 后 change-quality receipt cqr_b4def6dad00490f8583e 与 origin/main=16ee7e6c9、当前 diff 指纹一致且已验证有效。
我的整体评价
在 69d19988b 上,架构边界、typed state、authority semantics、domain-neutral error text 和 guidance-vs-obligation 均与现有 LoopX owner 对齐;新增的异常处理是局部、可回滚且覆盖了此前未测的输入边界。没有发现未解决的代码 finding。批准以两个条件为准:GitHub 新 head 的 required checks(尤其 Stage2C correctness、Python pytest、merge-gate)全部通过,并在 merge 前重新读取 exact-head readiness;若任一 required check 变红,应暂停合并并保留该失败证据。
English verdict: APPROVE — the malformed receipt is now a typed, path-free fail-closed outcome with a no-execution regression test; approve only after all required checks and exact-head merge readiness are green.
huangruiteng
left a comment
There was a problem hiding this comment.
Approval conclusion (author-owned PR; GitHub blocks formal self-approval)
动机
本 PR 的目标是把 Todo 的 caller-approved completion validation 绑定到任务实际所属仓库。原实现只有 Goal repository/cwd 这一默认路径,跨仓库 Todo 无法证明命令将在正确、隔离且干净的 checkout 中执行;同时,delivery-workspace receipt 中的非法 typed literal 会让 TypeScript decoder 的 RuntimeError 直接冒泡,既没有稳定的 lifecycle 状态,也可能让调用方误以为是内部崩溃。受影响的是 todo complete CLI、managed Turn writeback 和共享 completion transaction。当前 head 保留无 task_repository 时的 Goal repo 默认行为,只对显式跨仓库选择增加 receipt/worktree 约束。
改动思路
事务决策仍由 completion_transaction.ts 持有:它判断是否需要 validation、构造唯一的 caller_validation effect,并在 effect 返回通过 receipt 后才允许 durable completion。Python completion_validation.py 只是 host-effect adapter,复用既有的 capture_delivery_workspace、normalize_delivery_workspace_snapshot、canonical repository identity 和 caller-validation receipt schema。跨仓库路径要求 Turn-bound receipt、independent_git_worktree、canonical origin/identity 匹配和 clean git status;缺失、非法、冲突、脏或不可验证都在执行命令前返回 path-free typed failure。修复将 decoder boundary rejection 映射为既有 receipt 状态 workspace_receipt_invalid,不吞掉为成功,也不引入第二套异常协议。CLI 与 managed Turn 传入同一 effect 参数,前端/Lark 没有新增配置源。
具体改动
关键代码讲解
loopx/control_plane/todos/completion_validation.py::_resolve_completion_validation_workspace:无task_repository时直接保持 Goal repo fast path;跨仓库时先验证 receipt,再验证当前 worktree 的 canonical identity、独立 worktree 类型和 cleanliness。新增的窄try/except (RuntimeError, TypeError, ValueError)只包围 receipt decoder,把非法输入转为workspace_receipt_invalid,并明确保持local_path_captured=false。loopx/control_plane/todos/completion_validation.py::run_declared_completion_validation_effect:继续拒绝非caller_validation或非法 argv,将task_repository、Turn receipt 和验证 workspace 交给现有 runner;它不拥有 Todo policy 或 commit 决策。loopx/control_plane/todos/completion_transaction.ts::reduceTodoCompletionTransaction:把 canonical Todo 的可选、非空task_repository加入 effect,保留现有两次 reduction/一次 external effect 顺序。loopx/cli_commands/todo.py与loopx/cli_commands/turn.py:分别从 settlement writeback 或当前 exact Turn 捕获 workspace receipt,再调用同一 completion writer;没有新增 cwd CLI option。
测试新增了真实跨仓库 clean linked worktree 的 positive path,以及 missing/foreign/dirty/malformed receipt 的 fail-closed paths。畸形 receipt 回归明确断言 validator 调用次数为零、Todo 仍为 open、结果为 validation_blocked_completion 且不含本地路径。
对主干的风险
旧 review 的两个 P1 已处理并在最终 head 复核:
- malformed receipt 不再冒泡裸
RuntimeError,而是稳定的workspace_receipt_invalidtyped receipt;新的负向测试证明命令不会执行。 - 原先远端
stage2c-correctness-e2e的string_seqbarrier 失败在修复 headc4638719a上重跑通过;rebase 到最新主干后的新 head69d19988b仍在等待对应 workflow 完成,未对 shadow harness 做无关修改。旧失败因此更像 merge-ref/跨进程时序问题,而不是本 PR 的回归;最终以新 head 的 required check readback 为准。
本地证据:completion/lane Python 52 passed;shadow cursor 参数 7 passed;control-plane TypeScript full suite 1449 passed、1 skipped;npm run typecheck:control-plane 通过;Node 22.18.0 transaction tests 6 passed;changed-file Ruff、py_compile 和 public-boundary scan 通过。完整 mypy 在仓库基线上仍有 964 个既有错误,未作为本次代码回归。premerge canary 已执行完整风险集:direct checks、公共边界和 risk-profile smokes 通过;quota-plan-smoke 的 effect-runtime internal error 可在同一 origin/main 基线复现,因此作为明确环境基线失败记录,不绕过该结果。
变更范围为 11 个文件(+519/-39),所有运行时代码都有 CLI/Turn active caller;task_repository 是可选字段,默认关闭的 Goal repo 路径保持不变。rebase 后 change-quality receipt cqr_b4def6dad00490f8583e 与 origin/main=16ee7e6c9、当前 diff 指纹一致且已验证有效。
我的整体评价
在 exact head 69d19988b2aa237e4bcf7f781a715f4a05fa3c3b 上,架构边界、typed state、authority semantics、domain-neutral error text 和 guidance-vs-obligation 均与现有 LoopX owner 对齐;新增的异常处理是局部、可回滚且覆盖了此前未测的输入边界。没有发现未解决的代码 finding。批准以两个条件为准:GitHub 新 head 的 required checks(尤其 Stage2C correctness、Python pytest、merge-gate)全部通过,并在 merge 前重新读取 exact-head readiness;若任一 required check 变红,应暂停合并并保留该失败证据。
English verdict: APPROVE — the malformed receipt is now a typed, path-free fail-closed outcome with a no-execution regression test; approve only after all required checks and exact-head merge readiness are green.
Summary
task_repositoryProduct surfaces
todo complete --turn-instance-idconsumes the exact settlement writeback workspace receipt and validates the current worktreeValidation
PYTHONPATH=. /Users/bytedance/goal-harness/.venv/bin/python -m pytest -q tests/control_plane/test_todo_completion_validation.py tests/control_plane/test_completion_validation_lane_scope.py(51 passed)node --no-warnings --experimental-sqlite --experimental-strip-types --test tests/control_plane_ts/todo_completion_transaction.test.ts(6 passed)npm run typecheck:control-planepy_compile, andgit diff --checkPATHCloses #4307