Skip to content

fix(coordination): bind promotion recovery to reviewed policy - #4835

Merged
huangruiteng merged 5 commits into
mainfrom
codex/issue-4796-promotion-plan-identity
Sep 21, 2026
Merged

huangruiteng merged 5 commits into
mainfrom
codex/issue-4796-promotion-plan-identity

Conversation

@huangruiteng

@huangruiteng huangruiteng commented Sep 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • bind the reviewed coordination-authority promotion to one canonical plan digest covering the Goal, operation, selected canonical provider, exact shadow revision/projection, minimum operation count, and normalized required event kinds
  • persist that digest through the durable legacy-writer fence, promotion event, receipt, preview, and final readback so a fence-before-canonical interruption can only be recovered by the exact reviewed plan
  • keep legacy fences readable by the writer guard while rejecting any new promotion whose fence does not carry the matching plan binding
  • give the public-minimum Node compatibility job measured CI headroom without removing or narrowing any test
  • document the recovery identity in the bilingual shared-authority RFC

Problem and repair

The operator path merged in #4799 correctly recovered the exact operation after a durable fence was written and the canonical commit failed. However, the fence identity did not include minimum_operations or required_event_kinds. A retry could therefore keep the same operation and shadow head, weaken the qualification policy, and complete the irreversible cutover.

This PR moves those policy inputs and the selected target provider into a canonical promotion-plan digest owned by the existing TypeScript authority boundary. Both direct and reviewed promotion paths now require the durable fence to match that digest. The regression injects the real fence-to-canonical failure, proves that mutations to either policy input or the target provider fail closed without creating a canonical head, and proves that the unchanged request still recovers.

The required public-minimum Node job then exposed a stale 10-minute execution budget: current main and the repair head both reached the hard timeout while still passing tests, whereas the complete forward-runtime suite finished successfully in 11m05s. The job now has a bounded 15-minute timeout. Coverage, test selection, minimum Node version, and failure semantics are unchanged.

Validation

  • baseline regression reproduced on current main: changed qualification policy returned recovered after the durable fence
  • 67 focused TypeScript authority/provider tests passed
  • 113 Python CLI, contract, runtime-shadow adapter, provider-failure, delegation-preflight, and disposable file-store E2E tests passed
  • control-plane TypeScript typecheck passed
  • generated coordination contract check passed
  • git diff --check passed
  • standard premerge canary passed on the final local head: 10/10 catalog checks, 8/8 risk-profile smokes, public/private boundary scan passed, no failures or manual holds
  • CI budget evidence: public-minimum Node timed out at 10 minutes on both main@09f10c4e and the prior repair head; the complete forward-runtime suite passed in 11m05s

Safety and scope

  • preview remains effect-free and promotion remains explicit/default-off
  • no Dashboard, Lark, delegation, or read path gains promotion authority
  • no second authority writer or Python semantic owner is introduced
  • legacy source bytes remain unchanged in the interruption/recovery regression
  • the CI change only raises a timeout; it does not skip, shard, filter, or weaken the minimum-runtime suite
  • no credentials, private Goal state, local paths, or generated logs are included

This is a control-plane/runtime authority change and must be merged by an independent maintainer under the repository policy.

Closes #4796

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>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

动机

Issue #4796 要求把 legacy Goal 到 managed coordination-authority acceptance 的操作路径补完整。#4799 已经交付了主体流程,但它的最终精确-head 审查发现一个真实 P1:当 legacy writer fence 已持久化、canonical commit 尚未完成时,恢复请求只绑定了 operation 与 shadow source,没有绑定 minimum_operationsrequired_event_kinds 和目标 provider。于是,同一个不可逆 fence 理论上可以被一个弱化资格条件或改向 provider 的重试消费。

本 PR 在现有 TypeScript authority owner 内修复这条恢复契约。验收结果是:变更任何决策相关输入都 fail closed 且不创建 canonical head;只有完全相同的已审查请求能够恢复。该结果补齐 #4796 的剩余安全缺口,而不是另起一套 promotion 机制。

改动思路

权威输入仍然是现有 qualified shadow head、operation id、显式选择的 canonical provider、minimum_operationsrequired_event_kinds。PR 使用已有的 canonical JSON/bytes/SHA-256 工具,把这些输入归一化为一个 promotion_plan_sha256required_event_kinds 使用已有 Unicode 比较规则排序,避免仅由输入顺序产生误判。

这个 digest 是派生投影,不是需要人工同步的第二份事实。现有 runtime 是唯一的决策与副作用 owner:preview 只计算 plan;execute 将 digest 贯穿 durable fence、promotion event/receipt、结果和 readback;恢复和 direct promotion 都验证相同身份。旧 fence 仍可被 writer guard 读取,但由于无法证明完整 reviewed plan,不再能授权新的 promotion。

正向路径是“持久化 fence 后中断 → 使用完全相同请求重试 → fence/provider/plan 均匹配 → canonical authority 与 receipt 写入 → 返回 recovered”。负向路径把 policy、operation 或 provider 任一项改掉,均在 canonical 写入前返回 typed conflict/mismatch。

具体改动

精确 head 782ee4ace4fd6172a663c441def13e6c724b48b5 共改 9 个文件(+199/-21):authority 生产逻辑集中在两个既有模块;其余为聚焦回归、CLI/fixture 适配、中英文 RFC 契约披露,以及一项有运行证据的 CI 超时预算修复。没有新增服务、provider 抽象、Python 语义 owner、CLI 命令或手工维护状态。

关键代码讲解

  1. reviewed promotion plan 构造 在 preview/execute 分叉之前生成完整 plan digest,并把它同时装入 fence 与 direct request。这保证后续所有消费者看到同一身份。

  2. localCoordinationPromotionPlanSha256 是唯一归一化 owner:验证 Goal、operation、provider、shadow revision/projection 和 minimum,并对 required event kinds 去重、规范排序后进行 canonical hash。

  3. direct promotion 的 provider/plan 门禁 在任何 canonical mutation 前独立验证所选 provider、plan digest 和 durable fence bytes,分别返回 typed mismatch/conflict;不存在静默 fallback。

  4. legacy fence decoder 保留可选 digest 以兼容历史 writer guard。兼容读取不等于 promotion 授权:缺失 digest 会在 promotion 门禁处 fail closed。

  5. node-minimum-compatibility CI 门禁 保留 Node 22.18.0、typecheck 和完整测试选择,只把已耗尽的 10 分钟硬上限调整为 15 分钟。当前 main 与前一修复 head 都在 10 分钟处被取消,而同一完整前向运行时套件在 11 分 05 秒成功;最终 head 的最低 Node 套件也在 10 分 27 秒完整通过。这是让门禁产出真实结论的有界预算修复,不是放宽断言。

回归测试使用真实的 disposable file-store 中断点:先让 fence 落盘、故意使 canonical commit 失败,再分别改变 operation、provider、minimum 和 required event kinds,断言无 canonical head;随后用原请求重试,断言 recovered、receipt 匹配且 legacy source bytes 未改动。

对主干的风险

最强反对理由是:中断窗口较少,source revision/projection 已能阻止陈旧数据,新增持久化 identity 可能被视为过度设计。但基线复现证明 source 身份并不能表达“以什么资格政策、写入哪个 provider”这一审查决策;旧实现确实会让弱化 policy 的请求返回 recovered。把七项决策输入收敛到一个既有 owner 的 canonical digest,是比散落字段比较更小、也更不易漂移的修复。

默认关闭隔离保持不变:provider 可用、shadow qualified、CLI 可见或 preview ready 都不会激活 promotion;execute=false 不写 fence/canonical state。Dashboard、Lark、delegation 和 read path 均未获得 promotion authority。错误文本保持 Goal/provider 中立,规则由 typed request、canonical digest 和 reason codes 机器执行,并非“指导性”文字。

兼容性风险集中在历史 digest-less fence:它仍然阻止 legacy writer,但不能再被当作完整 reviewed plan 的证明。这是刻意的 fail-closed 行为;需要恢复此类旧中断时,继续走既有人工恢复边界,而不是猜测原审查意图。

CI 预算也单独做了 base/head 同工作量判断:10 分钟保护的是门禁最大耗时,但当前上限已不能让未删减的最低 Node 全量套件跑完;删测试或缩小选择会降低兼容性证据,继续保留 10 分钟则稳定取消。15 分钟在已观察到的 11 分 05 秒完整通过之上保留有限余量,同时仍显著低于 forward suite 的 20 分钟上限。

本地证据包括:在 origin/main@09f10c4e37b89c6ed25f2f2354f89d641662e83c 复现旧行为;最后一个 source-affecting head 80842b0a 上 67 个聚焦 TypeScript authority/provider 测试、113 个 Python CLI/contract/runtime-shadow/provider-failure/delegation-preflight/file-store E2E 测试、control-plane typecheck、generated contract check 与 git diff --check 全部通过。此后唯一改动是 CI timeout scalar 与说明;final head 再跑 standard premerge canary,10/10 catalog、8/8 risk profiles、public/private boundary clean、0 failure、0 manual hold。最终 GitHub CI 在本 head 上无失败,包含最低/前向 Node、Windows、Dashboard acceptance 与真实 PostgreSQL authority integration;release/presentation 的 skip 为工作流条件性跳过。

语义与 CI 对齐

该变更复用并收紧现有 Shared Goal authority-state provider v0 的 promotion/recovery vocabulary,没有创建平行语义或扩大 actor lifecycle。中英文 RFC 同步说明旧/新默认:过去相同 operation/source 可以携带变化后的 policy/provider 恢复;现在必须匹配完整 reviewed plan。当前仓库要求的 DCO、依赖审查、跨 Node/Windows/Python 测试、Dashboard acceptance、真实 PostgreSQL 集成和构建检查均在同一 head 上完成。

我的整体评价

没有 blocking finding。这个 PR 对原问题是完整且成比例的修复:它在正确的 TypeScript authority 边界内复用现有 canonical 工具,新增的是可推导 identity 而非第二权威源,并用基线失败与 exact-head 正/负路径证明行为变化。剩余风险已明确限于历史 digest-less fence 的人工恢复;这比在不可证明原 plan 时自动继续 promotion 更安全。

结论为 APPROVE,但这是 author-owned PR,GitHub 无法记录 formal self-approval,因此发布 COMMENTED exact-head review。另因改动触及 loopx/** control-plane authority 行为,仓库规则要求独立维护者完成 merge;本结论不构成自合并授权。

English verdict: APPROVE - exact head 782ee4a; no blocking findings, full plan/provider recovery identity and the evidence-backed minimum-Node CI budget are validated by baseline reproduction, focused local suites, premerge canary, and final green CI; independent maintainer merge is required for this control-plane authority change.

…otion-plan-identity

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>

@huangruiteng huangruiteng left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval conclusion (author-owned PR; GitHub blocks formal self-approval)

Reviewed exact head: ee564b32ed9307d59790f8310013387d0934b3cacodex/issue-4796-promotion-plan-identity,与 main 合并后重审)。

授权说明(写在最前):这是一次权威/权限边界变更(loopx/** 控制面),仓库规则要求由独立维护者合并,PR 正文也这么写着;本次是在 owner 明确指示下覆盖该规则后合并的。本记录的目的就是让这次覆盖可见,而不是把它藏掉。

动机

#4799 之后,durable fence 已经能精确恢复"fence 已落盘、canonical commit 失败"这个中断场景。但fence 身份里没有 minimum_operations,也没有 required_event_kinds:一次重试只要保持同样的 operation 与 shadow head,就可以削弱资格策略并把这次不可逆的 cutover 走完。失败形态很隐蔽——cutover 成功、receipt 正常,只有"评审当时依据的资格"被悄悄丢掉了。

改动思路

把资格策略与目标 provider 一起并入一个由既有 TypeScript 权威边界拥有的 canonical promotion-plan digest,并让这份 digest 贯穿 durable fence、promotion event、receipt、preview 与最终读回。direct 与 reviewed 两条路径都要求 fence 与 digest 一致。关键取舍:没有新增第二个身份来源——digest 挂在既有 fence 上(读取时可选,因此旧 fence 仍可被 writer guard 解析),而不是新开一个 policy 文件或第二个 store。

本 head 相对上次评审的唯一变化是与 main 的合并:唯一冲突是 minimum-Node lane 的 timeout-minutes: 15 上方那段说明注释(两侧都已改成 15)。我保留了本分支那段——它写的是"实测已超过十分钟",而这正是本 PR 的测量结论;main 那句"通常 9-10 分钟"与实测矛盾。

具体改动

9 个文件、+198/-23:

  • loopx/control_plane/coordination/local_authority_runtime.ts(+83/-1):新增 localCoordinationPromotionPlanSha256(schema loopx_local_coordination_promotion_plan_v0,绑定 goal、operation、canonical_authority、shadow revision、projection digest、minimum_operations、排序后的 required_event_kinds);reviewed 路径把 digest 写进 fence 与 preview plan;request 增加 canonical_authoritypromotionIdentity/promotionResult 带 digest;promoteLocalCoordinationAuthority 增加两个 fail-closed 守卫。
  • loopx/control_plane/coordination/legacy_writer_fence.ts(+9):promotion_plan_sha256 作为可选字段解码,缺失时不编造值。
  • TS 测试与 fixture(+91/-17):recovery 回归注入真实的"fence 成功、canonical commit 抛错"中断,并断言篡改被拒、原样可恢复。
  • tests/control_plane/test_sqlite_authority_cli.py(+1)、两份 RFC 镜像(+12/-2)、CI 注释(+2/-3)。

关键代码讲解

  • localCoordinationPromotionPlanSha256:把整份复核过的计划规范化后哈希。它是这次修复的核心——"哪些输入属于这次承诺"由这里单点定义;将来新增资格输入,必须加进这个对象,否则就落在绑定之外(这也是我保留的唯一真实风险,见下)。
  • promoteLocalCoordinationAuthority 的两个守卫:先比 request.canonical_authority 与打开后的 store provider 身份,再比请求重算的 digest 与 fence 里的 digest;两者都在任何 canonical 写入之前,失败时 legacy_writer_fenced: false(因为此时确实什么都没验证)。
  • reviewLocalCoordinationAuthorityPromotion:recovery 时仍要求新构建的 fence 与持久化 fence 逐字节相等,因此 digest 的不同会落成 local_authority_writer_fence_conflict——旧版本写下的 fence 与新 fence 永远不会相等,这正是 PR 明说的 fail-closed。
  • decodeLegacyCoordinationWriterFence:字段可选的写法让"旧 fence 仍能被 writer guard 读取"与"新 promotion 必须带 digest"同时成立。
  • 回归测试 reviewed promotion resumes the exact request after a fence-to-canonical interruption:断言被拒的每一次重试都让 canonical head 保持 missing、legacy 源字节不变,而原样请求恢复出的 head digest 等于 shadow projection。

我复核的关键点(都是自己跑的):

  • TS 五个模块 99 passed(runtime / provider / local shadow / coordination shadow / coordination state contract);单独跑 --test-name-pattern "reviewed promotion" 3 passed。
  • 变异验证(最关键的一条):把 minimum_operationsrequired_event_kinds 从 digest 里删掉后重跑 recovery 回归,测试失败(被削弱的策略被接受,也就是原始缺陷复现);恢复后同一测试通过、文件干净。这证明回归是承重的,而不是只为绿灯而写。
  • Python 六个模块 76 passed(本地协调权威、shadow adapter、sqlite CLI、shadow CLI E2E、bounded E2E、delegation preflight);npm run typecheck:control-planegenerate_coordination_state_contract.py --check 均 exit 0。
  • loopx canary premerge --from-git-diff:status passed、self_merge_allowed: truemanual_holds: 0、零失败。

对主干的风险

  1. 旧 fence 不再可被新 promotion 完成:这是有意的 fail-closed(PR 正文明确写了),但意味着"变更前写好 fence、变更后重试"的 operator 需要走 rollback/quarantine 路线。我没有在 head 上端到端跑过那条路线,这一点我标为未验证维度,而不是替它背书。
  2. 未来资格输入可能落在绑定之外:如果将来给 request 加了新的资格字段却忘记加进 digest 对象,绑定会静默变窄。今天的两个输入由变异验证覆盖;localCoordinationPromotionPlanSha256 是唯一需要改的地方。
  3. 没有 PostgreSQL 实机 promotion:守卫的 provider 行为靠 file-store fixture 与 CI,属证据缺口而非结论。

除此之外,promotion 仍然 explicit/--execute 门控、preview 无副作用、没有任何读路径或 dashboard/Lark 路径获得 promotion 权限,也没有引入第二个 writer。

我的整体评价

结论 APPROVE。这次把"一次不可逆 cutover 只能被精确复核过的计划恢复"从部分绑定补成完整绑定,并且用真实中断注入 + 变异验证证明了回归是承重的;实现没有新增第二个身份来源,旧 fence 仍可读、但不能授权新的 promotion,方向正确。上面三条风险都是边界性的、可陈述的,其中第一条属于本 PR 明示的取舍。

English verdict: APPROVE - exact head ee564b3 (re-review after merging main; the sole conflict was the explanatory comment above the minimum-Node timeout, where I kept this branch's measured wording). The durable fence identity now covers the complete reviewed plan: goal, operation, selected canonical authority, exact shadow revision and projection digest, minimum operation count and normalized required event kinds, all hashed by localCoordinationPromotionPlanSha256 under loopx_local_coordination_promotion_plan_v0 and persisted through the fence, the promotion event, the receipt, the preview and the readback, with both the direct and reviewed paths requiring the fence to match. The fence field is optional on decode so legacy fences stay readable by the writer guard, while a new promotion whose fence lacks the digest fails closed. I verified the regression is load-bearing by removing minimum_operations and required_event_kinds from the digest in a scratch copy: the recovery test then fails because the weakened policy is accepted, and passes again after restoring. 99 TypeScript authority/coordination tests, 76 Python CLI/contract/shadow/delegation tests, the control-plane typecheck and the generated coordination contract check all pass, and canary premerge reports passed with self_merge_allowed true and manual_holds 0. Three limits are recorded rather than hidden: a fence written before this change can no longer authorise a promotion through these paths, so the operator route is the rollback/quarantine path, which I did not execute end to end; a future qualification input added to the request but not to the digest object would fall outside the binding; and no PostgreSQL-backed promotion was run at this head. Authority note: this is a permission/authority-boundary change under loopx/**, which the repository policy leaves to an independent maintainer and which the PR body itself says must be merged by one - the merge was performed under the owner's explicit instruction to override that rule, and this review records the override so it is visible.

@huangruiteng
huangruiteng merged commit 7e64ade into main Sep 21, 2026
14 of 20 checks passed
@huangruiteng
huangruiteng deleted the codex/issue-4796-promotion-plan-identity branch September 21, 2026 07:02
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Post-merge self-repair audit

  • I found no new blocking defect in the business logic shipped by exact head ee564b32ed9307d59790f8310013387d0934b3ca.
  • The merge-process signal was unsafe: loopx canary premerge could report self_merge_allowed: true from validation success alone, even though this loopx/** control-plane change still required final CI and an independent maintainer under repository policy.
  • This PR was merged before final CI completed; several later jobs were cancelled, so the merge itself is not evidence that the final exact-head check set passed.

Durable repair: #4853 keeps the legacy authority-shaped field fail-closed, reports validation separately, and points consumers to repository policy plus exact-head merge readiness. Its focused smoke covers both the green-validation case and a later enforced quality-receipt failure.

#4853 will not be author-self-merged because it also changes loopx/** control-plane behavior; after its exact-head CI and published review, an independent maintainer must perform the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(delegation): add reviewed operator path from legacy Goal to managed-worker acceptance

1 participant