Repository navigation
fix(delegation): preserve preflight rejection reasons - #4757
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
Reviewed exact head 098066619de810156661e0051376ca19488034b8. I found one blocking P1 consumer-contract regression.
动机
这个 PR 要解决的 backend 问题成立:canonical authority 缺失不应迫使 inspect 猜测/启动 provider,Turn plan 的安全 rejection 也不应在 Python 先读 transaction.turn_key 时退化成 KeyError。把这些分支交给 TypeScript decision boundary 归一化、Python 只提供 host facts 和捕获异常,是正确方向。
但 PR body 同时声明 shared delegation read model 会被 CLI、managed Turn 和“现有 HTTP/frontend consumer”共同消费,并据此判断无需前端 companion change。exact-head 调用链证明这项声明不成立:新 backend response 扩展了状态词汇和字段 nullability,而 active frontend 仍按旧闭集读取。
改动思路
backend 分为两个清晰决策:
delegationPreflight接收 canonical authority readiness;缺少 authority 时返回state="authority_unavailable"、turn_route=null、executor=null、authority_reason=...,且四类 effect 全为 false。delegationTurnPlanDecision在 host adapter 读取 transaction 前区分 rejected/planned;只有ok=true且turn_key满足sha256:<64 hex>才允许进入 running。Delegations.inspect/_execute负责采集 authority/CLI 结果和持久化原 operation 的 rejection,不提升 authority、不创建 replacement Turn。
这些 backend 边界本身是合理的;问题在于 shared read model 没有端到端完成到现有浏览器 consumer。
具体改动
关键代码讲解
delegationTurnPlanDecision(delegation.ts:44):保留plan.error/reason,成功分支严格校验 turn key,避免 Python 重新发明 Turn plan 语义。delegationPreflight(delegation.ts:64):新authority_unavailable分支正确禁止 preview/acceptance/effects,并给出显式 repair guidance。Delegations.inspect(collaboration_mcp.py:165):把 missing canonical authority 从 raw exception 变成 typed observation;再次核验 binding,避免 source drift。Delegations._execute(collaboration_mcp.py:397):先调用 typed decision,再读取turn_key;outer operation wrapper 会把 rejection 写回原 operation。- 遗漏的 active consumer:
chat.ts:898的DelegationPreflightunion 没有authority_unavailable,仍声明turn_route: string和非空executor;goal-team-work.tsx:60-62随后无条件读取executor.profile/host/reason。
对主干的风险
[P1] 新 missing-authority 路径会让 Team execution 面板崩溃,而不是显示 repair reason
触发条件就是本 PR 要改善的场景:Goal 已配置 delegation binding,但尚未 promote canonical authority。
实际链路:
GoalTeamWork 点击“检查启动条件” → HTTP operation=inspect → Delegations.inspect 捕获 missing authority → delegationPreflight 返回 {state:"authority_unavailable", turn_route:null, executor:null, authority_reason:"..."} → React render 读取 executor.profile(随后还读取 host/reason)。
错误结果:用户看不到“显式修复/promote authority”的安全解释,反而遇到 render exception/空白面板。因为 requestJson<T> 只是泛型约束而不是 runtime schema parse,TypeScript build 也不会从真实 JSON 自动发现这个不一致;这解释了为何 backend tests 和 CI 都是绿的。
最小修复:
- 将
DelegationPreflight.state扩展为包含authority_unavailable;把turn_route、executor按 response contract 改为 nullable,并加入authority_ready/authority_reason。 GoalTeamWork对authority_unavailable显示中英文 label 和authority_reason,只有executor !== null时才读取 profile/host/reason。- 增加一个 component 或 packaged browser regression,使用 exact response
{state:"authority_unavailable", turn_route:null, executor:null, authority_reason:"..."},断言不崩溃、显示 repair guidance、没有 launch/effect。
验证结果:
- source-checkout provenance:
.venv/bin/python+ exact-headloopx/__init__.py。 pytest -q tests/test_delegation_preflight.py:18 passed。node --test tests/control_plane_ts/delegation.test.ts:7 passed。git diff --check:通过。- GitHub exact-head checks:28 个 success(含 real PostgreSQL authority、adapter contract、merge gate);release/publish jobs是 intended skips。
这些证据充分证明 backend fix,但也明确暴露 validation gap:没有任何测试跨过 changed response → HTTP/frontend → React renderer 这个真实边界。green CI 不能覆盖上述具体反例。
我的整体评价
结论是 REQUEST_CHANGES。核心方向、typed state rule、authority semantics 和代码比例都合理;我不建议退回 Python catch-only 或自动 promotion。阻塞点是一个很小但必须同 PR 修完的 product-journey 完整性问题:既然 schema version 仍是 loopx_delegation_preflight_v0、PR 又明确承诺现有 frontend consumer 可直接消费,那么新增状态和 nullability 必须原子更新所有 active consumers。
未来向 pass:修复应复用现有 DelegationPreflight/GoalTeamWork owner,不需要新 adapter、新状态框架或另开 PR。完成上述三项并重跑真实 response-shape 场景后再复审。
English verdict: REQUEST_CHANGES (author-owned fallback: COMMENTED). The backend now preserves safe authority/Turn-plan rejections correctly, but the new authority_unavailable response sets executor=null while the active Dashboard contract excludes that state and dereferences executor.profile/host/reason; update the shared consumer and add an exact-response frontend regression before merge.
Exact head: 098066619de810156661e0051376ca19488034b8
|
Addressed the frontend parity blocker in What changed:
Validation:
Entry points: CLI/managed Turn remain covered by the existing typed preflight response; the dashboard now consumes that same projection safely. Lark receives the shared receipt/projection and has no separate control or source of truth in this slice. This remains a control-plane change and is intentionally left review-required for maintainer merge. |
|
Follow-up: the first refreshed-head CI correctly caught that the packaged Dashboard bundle had not been committed. I rebuilt from the same source and pushed signed commit |
00f6a1a to
e61dfb3
Compare
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
e61dfb3 to
a01444d
Compare
Summary
authority_unavailablereadiness without inspecting or launching a providerKeyErrorProduct surfaces
The shared delegation read model changes for CLI, managed Turn, and the existing HTTP/frontend consumer. No new frontend or Lark control is required: both already consume this projection, and the change adds a typed blocked state rather than a new setting or action.
Validation
uv run --extra test python -m pytest -q tests/test_delegation_preflight.py— 18 passednode --no-warnings --experimental-strip-types --test tests/control_plane_ts/delegation.test.ts— 7 passednpm run -s typecheck:control-plane— passedloopx canary premerge --from-git-diff— standard tier passed, 10/10 selected checksok=true,state=authority_unavailable, and all effects falseThe full
npm run test:control-planerun exposed an existing local toolchain issue: subprocess tests resolved Apple Python 3.9, which does not supportdataclass(slots=...). Re-running the affectedtodo_continuation.test.tswith the project Python 3.13 first on PATH passed 36/36.Boundaries