fix(monitor): unify successor routing before writeback and receipt validation - #4145
Conversation
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
Signed-off-by: huangruiteng <huangrt01@163.com>
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
审阅绑定 exact head:935ebb805db704eeaf3d9a0f991a6abffac45f14。
动机
把 monitor successor 的 preflight、Python writeback 和 provider-receipt validation 收束到一个 typed route owner,方向和必要性都成立。它能避免 action、capability、repository 与 receipt 在三层各自解释后漂移,也保持了“typed TypeScript 决策、Python host effect”的既有边界。
改动思路
新的 monitor_successor.ts 统一校验 successor intent、规范化 route,并由 quota preflight 与 receipt validation 直接复用;Python adapter 通过 effect runtime 取得同一个计划后再执行现有 Todo 写回。原 observation 继续参与 v0 fingerprint/provider plan,canonical route 仅用于物化与回读比较,这个兼容策略本身合理。
具体改动
关键代码讲解
monitorSuccessorIntent统一约束 material change、action、claim、continuation、capabilities、repository 与 user-task class。monitorSuccessorRoute生成 provider writeback 和 receipt comparison 共用的 canonical successor route。write_monitor_poll_todo_state删除 Python 的 route 决策重复,只保留 source lookup 与实际 effects。validatedProviderReceipt复用同一个 typed route,不再单独实现 defaults/capability comparison。
但这里有一个阻塞的边界遗漏:monitor_successor.ts 的 URL 校验只检查 url.password,没有检查 url.username;保留的 repository_identity.py 也同样只检查 password。现有测试只覆盖 https://user:password@...,因此全部通过时,username-only credential 仍会漏过。
我通过真实 public CLI 路径复现:在临时 synthetic registry 上执行 loopx quota monitor-poll --execute,传入 https://user_only@example.invalid/owner/repo,命令成功并创建了 git:example.invalid/owner/repo successor,而不是在第一次写入前拒绝。这与 PR 声明的 credential-free、pre-write rejection 语义冲突。
最低修复范围:
- 在 TypeScript 与保留的 Python codec 中都拒绝 HTTP/HTTPS/git 的 username credential;若契约需要
ssh://git@host/...,要用 transport-aware 规则保留合法 SSH username。 - 增加 public
quota monitor-poll负向回归,证明 username-only credential 在 provider plan/首次写入前失败且 Todo/receipt 状态不变。 - 增加两套 codec 的 parity case,防止其中一侧以后再次漏掉该语义。
验证结果:27 个聚焦 Python 测试、20 个 TypeScript 测试、Ruff、git diff --check、DCO 与 public/private 扫描均通过;上述真实入口反例单独失败并构成阻塞证据。
对主干的风险
这是公共控制面输入边界。把 credential-bearing URL 静默剥离成 canonical identity 会让调用方误以为不安全输入已被合规接受,也破坏 provider plan/fingerprint 所承诺的 credential-free 约束。影响只需一个小而明确的 admission 修复即可消除,不需要推翻本 PR 的架构。PR 当前还 BEHIND main,修复后应一并 rebase 并重跑检查。
我的整体评价
整体重构是 proportionate 的,production net change 也很克制,typed owner 与 host effect 分工清晰;但本 PR 最强的承诺之一正是“所有不可表示/带 credential 的 route 在 writeback 前拒绝”,而当前测试 oracle 漏掉了 username-only URL 这一等价输入形态。修复该语义差异并补齐真实入口负向回归后,可以快速复审。
English verdict: REQUEST_CHANGES — exact head 935ebb805db704eeaf3d9a0f991a6abffac45f14 accepts username-only HTTP credentials at the public monitor-poll boundary; reject them in both reachable codecs and add no-write parity coverage.
| try { | ||
| const url = new URL(raw); | ||
| if (!["git:", "http:", "https:", "ssh:"].includes(url.protocol) || | ||
| !url.hostname || url.password || url.search || url.hash) throw new Error(); |
There was a problem hiding this comment.
[P1] url.password does not cover username-only HTTP credentials. The real quota monitor-poll --execute path accepts the synthetic https://user_only@example.invalid/owner/repo input and creates a successor instead of rejecting before writeback. Please reject url.username for non-SSH transports in this typed codec and the retained Python repository codec, preserve legitimate SSH usernames deliberately, and add a public-entrypoint no-write parity regression.
Summary
Unify monitor successor routing across quota preflight, legacy writeback and provider-receipt validation. This is a bounded T2 prerequisite against the TypeScript/control-plane and shared Goal Authority RFCs, not a new monitor engine or an atomic native writer.
scheduler/monitor_successor.tsowner, reused directly by quota and through the existing effect runtime by the Python writeback adapter.Issue Or Task
Maintainer-requested next cohesive RFC implementation slice. Started independently of #4142/#4143; rebased onto merged #4142 and revalidated its authoring scope. #4143 is not a prerequisite.
Intentional semantic changes
next_claimed_byis treated like other agent-route flags and requires an agent successor.Unchanged: material-change generation, result-hash successor keys, unchanged polling, user_action/user_gate distinction, authoring/claim authority, user approval/global-gate scope, writer fences, provider defaults and promotion holds. A valid route is not an execution grant. This does not promise that unrelated downstream authority/effect failures cannot leave a partially completed legacy workflow; full monitor-plus-successor atomicity remains T2 work.
Validation
935ebb805db704eeaf3d9a0f991a6abffac45f14, based onbc18304a02acabc3f7c8c55728743885c0c24f91.npm run test:control-plane: 942 passed, no failures or skips, with PostgreSQL enabled.quota monitor-poll --executewith alias action/Git URL/capability inputs produces a canonical independent advancement successor; invalid successor intent leaves the monitor document unchanged.python examples/control_plane/monitor-poll-writeback-smoke.pycompleted successfully on the rebased implementation.Coverage and gaps: validation exercises the actual CLI and existing legacy effects, typed preflight/receipt lifecycle, retry/CAS behavior, negative authority boundaries and realistic fixture structure. Earlier development failures included an incorrect hash test vector, a conflict-result assertion expecting an exception, and a TS nullable-union narrowing issue; these were corrected before final validation. The final commit adds only the production-scale test; production code is unchanged from the rebased Python/smoke runs. No live Goal was mutated, and no full-repository Python or whole-Goal promotion qualification is claimed. Hosted CI will run separately.
Type of Change
LoopX Area
Technical Direction
mainShared-authority RFC fixture impact
loopx_coordination_production_scale_fixture_v0; existing public fixture reused unchanged.Boundary Checklist
Future-facing pass applied: one route owner is ready for the eventual native T2 transaction to call in process. The node-independent repository/bootstrap codec is intentionally retained and characterized rather than making bootstrap depend on Node. T1 update closure, full T2 atomic writeback/recovery and shared-provider durability qualification remain separate, explicit work.