refactor(coordination): make Goal Channel ownership provider-aware - #4316
Conversation
huangruiteng
left a comment
There was a problem hiding this comment.
Request changes conclusion (author-owned PR; GitHub blocks formal self-review)
动机
本评审针对 exact head 0f5b844cb2a5af39bf9cae67a8365aa7bdea290c。PR #4316 解决的是 Goal Channel 所有权展示的 authority 漂移:原实现把 lease 的过期、代数和冲突判断留在 Python display projection,并直接读本地 lease 文件;canonical provider 晋升后,这条旁路可能继续展示陈旧 ownership。当前改动把完整 Todo/lease revision 的读取和判断收敛到 TypeScript provider boundary,同时保留晋升前 legacy adapter。这样 active_leases 仍是只读展示,不会被误解为 claim、lease 或 mutation 授权。
改动思路
入口仍是 build_goal_channel_projection。observe_goal_coordination 根据既有 promotion fence 选择 canonical local-authority effect,或在晋升前收集状态 Todo 与本地 lease 文件;两条路径都进入 projectOwnershipObservation,共享一个观察时间、lease 活跃性、代数、冲突和 100 条展示上限。canonical 路径先加载并验证 AuthorityStore 的 Todo read model,再构造完整观察,最后将诊断行置前并截断。provider 失败变成 coordination_unavailable,不会回退 Markdown 或本地旧 lease;明确的 active_leases=[] 也不再推断 soft claim。Python 只做 source selection、输入适配和既有文本脱敏,TypeScript 继续拥有 typed state rule,符合现有 provider/authority 边界。
具体改动
关键代码讲解
loopx/control_plane/coordination/ownership_observation.ts:25-65的projectOwnershipObservation验证观察时间,区分 explicit 与 derived claim,统一处理 active lease、owner/claim 冲突和损坏 lease;它只返回安全字段,不复制操作 key、write scope 或后端私有字段。loopx/control_plane/coordination/ownership_observation.ts:68-83的readCoordinationOwnership从选定AuthorityStore做一次完整 read,复用indexCoordinationProjection/validateCoordinationTodoReadModel,先完成完整性判断再做显示截断,并附带 provider revision/cursor。loopx/control_plane/coordination/local_authority_runtime.ts:1210-1229与effect_runtime_handlers.ts:481-485注册 canonical read-only effect;coordination_observation.py:12-56在 promotion 前后选择来源,canonical 异常时 fail closed。goal_channel_projection.py:350-446删除旧的 Python lease decision/read path,把结果投影为 active_leases 与 coordination_observation/source_warnings;HTML renderer 仅根据 unavailable 状态改变空态颜色和文案。
对主干的风险
聚焦验证通过:canonical/legacy Goal Channel 与 hard-lease Python 测试 17 个通过;扩大后的 handoff/status/lease Python 集合 73 个通过;ownership、provider failure matrix 与 AuthorityStore conformance Node 集合 38 个通过;py_compile 与 git diff --check 通过。覆盖了 provider 空值、不回退、显式空列表、过期/损坏 lease、冲突、截断、隐私字段过滤以及现有 renderer。npm run typecheck:control-plane 因隔离环境没有 tsc 退出 127,属于验证缺口。
唯一阻塞项来自 exact-head merge state:GitHub 对 0f5b844cb2a5af39bf9cae67a8365aa7bdea290c 报告 mergeable=CONFLICTING、mergeStateStatus=DIRTY,虽然 28 个 checks 全部成功。这意味着 PR 当前不能合并,且冲突解决后的代码尚未经过这些 checks;现有绿色结果不能替代新 head 的验证。请先以当前 main 为基线 rebase/merge、解决冲突,再重新运行 required checks 和本评审的 provider conformance 集合。
我的整体评价
实现方向和责任边界是正确的:canonical provider 成为晋升后的唯一 ownership read source,legacy 路径保持 default-off,展示层不再拥有 lease 规则,且 docs/中英文 RFC 明确了 unavailable、truncated、explicit-empty 和只读 authority 语义。469 行新增/142 行删除属于一个围绕同一 observation boundary 的适度多面变更,没有发现独立的运行时逻辑 blocker。当前 exact head 仍需 REQUEST_CHANGES,原因是 GitHub 的真实合并冲突以及 tsc 环境缺口。冲突修复后请以新的 exact head 重新取证;若新 head 保持行为与全绿 checks,可再行批准。
English verdict: REQUEST_CHANGES for exact head 0f5b844cb2a5af39bf9cae67a8365aa7bdea290c; implementation-focused tests pass, but GitHub reports CONFLICTING/DIRTY, so the PR cannot merge and the resolved head still needs full revalidation.
caa6214 to
38b9fef
Compare
|
Final rebase/refine completed on pushed head Changed surfaces: canonical Goal Channel ownership observation, typed TS provider/read boundary, Python projection adapter and renderer failure feedback, conformance fixtures, and bilingual RFC/reference docs. Final checks:
The full Python repository suite remains intentionally outside this merge gate because it is unbounded; no failure was observed in the selected changed-surface suite. No private data, credentials, local runtime state, Todo, lease, or receipt was written. |
38b9fef to
a24dea4
Compare
Rebase / self-repair / final validation (2026-09-14)
Validation:
No private logs, credentials, local paths, or tracked runtime state were added. The full unbounded Python repository suite is outside this PR's selected gate; changed-surface tests, real File/SQLite/PostgreSQL paths, loopx-meta rehearsal, and pinned canary cover the touched behavior and boundaries. |
a24dea4 to
bc6abfb
Compare
Final rebase update
The pinned canary and quality receipt were refreshed for this exact pair:
The rebase preserved the same 18-file scope and signed three-commit history. GitHub CI for the preceding equivalent head was fully green (all blocking jobs passed; only workflow-conditional skips and non-blocking Sonar pending). The final head is now queued for the same required checks. |
…apshots 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>
bc6abfb to
412126b
Compare
Self-review conclusion after final rebaseReviewed the exact final diff ( Local evidence for this exact scope: TypeScript typecheck; 1,535 control-plane tests passed with one expected PostgreSQL-unconfigured skip; 88.50% statements; mypy and Ruff passed; 37 focused Python tests; 102 isolated PostgreSQL integration tests; read-only loopx-meta three-arm rehearsal passed with source unchanged, exact provider heads/receipts and legacy semantics; pinned canary passed (17 selected/executed, 0 failures, 0 manual holds); exact receipt The final head was force-pushed with lease after the last main advance. Per the requested workflow I am not waiting for the new head's GitHub CI before attempting the merge; repository policy still forbids bypassing any required check that is pending or failing. |
Merge gate resultSelf-review is complete and the local exact-scope gate is green. I attempted normal squash merge, but GitHub rejected it because the final rebased head still has required checks in |
Summary
This PR is rebased onto the current
mainand makes Goal Channel ownership observation provider-aware. It closes the read boundary around the canonical authority snapshot while preserving legacy file-backed behavior for non-promoted goals.Semantic changes and fixes
coordination_unavailablewith warning feedback; there is no silent legacy fallback. Healthy empty ownership remains distinct from unavailable ownership.This is a read-only observation refactor. It does not change the default provider, create receipts, repair Markdown, send messages, expose PostgreSQL selection, or authorize whole-Goal cutover.
Scope
18 files, 492 insertions and 142 deletions: bilingual RFC updates and a public operation guide, two synthetic guide images, the typed ownership observer/effect wiring, Python projection and renderer adapters, and focused TypeScript/Python conformance tests.
Validation
npm run typecheck:control-plane— passed.22.22.3/ SQLite3.51.3:npm run test:control-plane— 1,535 passed, 0 failed, one expected PostgreSQL-unconfigured skip; coverage run passed with the same result and 88.50% statements.python -m mypy— passed for the configured 22 source files.loopx-metathree-arm rehearsal — passed: source unchanged, provider heads/receipts exact, legacy semantics exact (310 Todos, 11 leases observed).loopx canary premerge --from-git-diffagainst pinned base9e2b6d425fff9b8b2ebd69a79c3d8491b12eb7b3: 17 selected/executed checks, zero failures, zero manual holds; public-boundary scan clean for all 18 changed files.loopx-metadiagnose/status/history/quota checks completed against the local runtime; no goal state, registry, Todo, lease, or receipt was written.The unbounded full Python repository suite was not used as a merge gate; the changed-surface tests, static checks, real File/SQLite and PostgreSQL paths, and repository canary are the selected evidence for this PR.
Self-repair and review hygiene
The original request-changes state was caused by an exact-head/base conflict and stale validation assumptions. The branch was rebased onto current
main, the three conflict files were resolved, and a bounded refinement pass fixed two issues found during review: truthfulcanonical_unavailablesource labeling and public-field redaction for explicit entries. All commits carry DCO sign-off.Exact-scope quality receipt
cqr_d7c7bc392244864075d4verifies valid for the pinned base/head diff; the one policy-authorized safe-fix pass is recorded. The PR remains independent of the ongoing SQLite default/cutover work; it supplies the provider-neutral observation seam that future File/SQLite and PostgreSQL selection can consume.