Skip to content

Make event-owned Todo completion atomic and source-bound - #5003

Merged
huangruiteng merged 2 commits into
mainfrom
codex/event-completion-transaction-0924
Sep 25, 2026
Merged

huangruiteng merged 2 commits into
mainfrom
codex/event-completion-transaction-0924

Conversation

@huangruiteng

@huangruiteng huangruiteng commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem and result

Completing an event-owned Todo previously appended successor add/claim events before it encoded the parent's completion event. A late encoding error left a runnable orphan. A source change after the completion context was read could still return success. Both counterexamples fail against the base and pass here.

This PR plans the full event batch in one TypeScript admission step, compares the source checksum under the event lock, and publishes all successor and completion events with one durable atomic replace. Exact replay confirms durability without creating another successor. Existing capture gates remain closed until the event writer is wired to the prepared/committed outbox lifecycle.

Reviewable changes

  • goal.state_event.plan_append owns batch conflict/replay and sequence allocation; Python retains the legacy event codec, lock and filesystem effects. Cross-language transport carries only the source witness and relevant identities.
  • Event completion encodes existing typed successor proposals and commits the whole batch once. Redundant Python successor default/normalization/write decisions are removed. The public completion result retains empty list fields.
  • Eager list/tuple batches become atomic; lazy iterables retain their prior per-item/reentrant contract. Historical JSONL bytes and schema remain unchanged. Physical file replacement means readers must reopen the log.
  • A narrow source projection readback type fix clears a pre-existing strict mypy error without changing its runtime validation.
  • Bilingual RFC ledger reconciles refactor(authority): unify complete source capture in TypeScript #4967 source assembly and refactor(authority): deliver outbox entries through one typed recovery owner #4968 delivery as already done, and gives the conditional 5–8 package path to a qualified File/SQLite default. It explicitly retains the event-writer capture and D2/D3 holds.

Validation and limits

  • Public Todo completion counterexamples fail on base and pass on this head. Current-base focused Python tests: 138 passed; broader mixed-source/capture tests on the immediately preceding base: 161 passed. Typed planner/dispatch tests: 17 passed. Relevant real-path supervisor and downstream read smokes passed.
  • TypeScript typecheck, strict mypy (23 files), Ruff, bilingual docs governance and public boundary scan passed. Quality receipt: cqr_6fa14563d596ed6b36b1 for the rebased exact diff.
  • Read-only source-copy rehearsal used a real 6.1 MB Markdown snapshot with 874 backfilled events. A disposable registry and real CLI completed one synthetic Todo, created its successor, replayed with unchanged bytes and confirmed the source digest unchanged. No live Goal was promoted or rewritten.
  • Same 707 KB detached log, seven warm three-event batches: median 11.33 ms on base and 28.94 ms here. The extra cost is whole-log atomic publication plus durability. This legacy adapter is not the future high-throughput provider and should retire with its final caller.
  • The unscoped local full TypeScript run selected system Python 3.9 for some subprocesses and produced unrelated failures. A controlled run with this checkout's supported Python and bounded file concurrency was interrupted before completion; the focused tests and premerge suite passed, and PR CI remains the full-suite oracle. The pre-existing maintainability ratchet finding in an unchanged Lark module reproduces on base; the premerge gate classified it as inherited and passed: 19 selected checks, zero failures or manual holds.

Runtime changes are limited to the legacy event-owned Todo completion and event append adapter. File/SQLite/PostgreSQL authority stores, provider defaults, frontend settings and persisted event schema are unchanged. The immediate successor is event-writer/outbox binding and mixed-writer whole-Goal rehearsal; SQLite D2 qualification stays with #4224. This PR closes one prerequisite within that package, not a complete migration stage.

@huangruiteng

huangruiteng commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator Author

Premerge evidence for head 710b262f2bfcb5b674018fa4cc4b0477a13c381f:

  • Changed surfaces: legacy event-owned Todo completion, the TypeScript append planner, Python event-log publication, focused tests and the bilingual RFC checkpoint. File/SQLite/PostgreSQL authority stores and frontend settings are untouched.
  • loopx canary premerge --from-git-diff --goal-id loopx-meta: passed, 19 selected checks, zero failures, zero manual holds. Strict change-quality receipt cqr_6fa14563d596ed6b36b1 verifies against this exact base/head scope.
  • The repository-wide maintainability ratchet reports an unreviewed ceiling in loopx/extensions/lark/goal_topic_runtime.py. The same finding reproduces on base; premerge classified it as an inherited advisory because this diff does not touch that module. I did not raise the ceiling.
  • Focused current-base Python tests: 138 passed; typed planner/dispatch: 17 passed; TypeScript typecheck, Ruff, mypy, docs governance and public boundary scan passed. An isolated real-source-copy CLI completion/replay passed without writing the source Goal. The qualified full TypeScript run was interrupted before completion; remote CI is still running.

These checks cover the changed transaction, lost-ack retry, concurrent source drift, event-only capture hold and source-copy readback. This PR is left for maintainer review; it does not grant provider promotion or remove the event-writer capture hold.

@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)

动机

审查对象:PR #5003,精确 head 710b262f2bfcb5b674018fa4cc4b0477a13c381f,基线 bdf1519cfe4a4d044dcb2eb8091a6242bd178173。事件源 Goal 的 Todo 完成旧路径按顺序写 successor add/claim,再写父 Todo completion;后段编码或源状态变化时可能留下可执行的孤儿 successor。接受标准是同一源版本下整组事件要么一起持久化,要么完全不发布,并在不确定提交后可安全读回与重试。

改动思路

完成入口先由既有 TypeScript successor owner 派生提案,Python 仅编码 add/claim/completion 事件;AppendOnlyStateEventStore.append_many 在事件锁内比对投影 checksum,让 TypeScript planner 决定冲突、重放与连续序号,再通过临时文件替换和 fsync 一次发布完整 JSONL。文件/SQLite/PostgreSQL 主权写入路径与前端配置未改,event-writer capture hold 仍在。

具体改动

  • event_writeback.py(约 267、481、536、614 行)把 successor 的编码与写入分开,并把全部事件连同父完成事件交给一次 source-bound append;空批次 replay 也校验源并补做耐久确认。
  • state_event_append.ts(25–67 行)处理源摘要、同 ID 冲突、同批重放与序号分配;event_sourced_state.py(594–712 行)保持旧行字节不变,锁内一次发布并把 fsync 后不确定状态显式报出。
  • 合成的相同输入经真实 store 路径对比:批次第二个事件无效时,基线抛错但留有第一个事件;此 head 抛错且日志未创建。精确 head 的 Todo 完成/并发源漂移/丢失 fsync ACK/重放以及共享 store 用例本地 90 passed;TS planner 6 passed,Ruff 与 diff check 通过。

对主干的风险

未发现可复现的代码阻断项。最大权衡是所有 append(不仅 Todo 完成)现在也会整文件替换:旧 inode 不再持续增长,长期 tail 读者需重开;PR/RFC 已明确披露,仓库内共享消费者检索未发现保持打开的读句柄。RFC 的同负载样本约从 11.33ms 增至 28.94ms/批;这是为耐久原子性支付的成本,应在日志继续增长时监测。当前 GitHub 精确头仍有失败的测试/合并检查;本次 review policy 不以远端 CI 替代本地代码证据,也不据此放行合并,维护者需查明并修复/重跑这些门禁。完整仓库测试和完整 TS suite 未在本次独立复跑。

我的整体评价

APPROVE(代码审查结论,不是合并许可)。 该改动击中了经基线/head 反例验证的孤儿 successor 问题,复用了既有 Todo/事件 owner,正反路径与重试读回有定向覆盖,未扩大 capture 或 Agent 权限。相邻的未来向整理已体现在 TS planner 单一决策 owner 与 Python 编码/IO 分工;进一步抽离通用原子文件写入或改进大日志成本可在有新需求/实测瓶颈时处理,不应阻断这个完整切片。控制面 PR 按仓库规则交维护者合并;失败的必需检查在合并前仍需解决。

English verdict: APPROVE - exact head 710b262 fixes the reproduced eager-batch partial-publication defect, with focused local Python/TypeScript validation and source-bound retry coverage. This is a code-review conclusion only; maintainers must resolve failing required checks, and control-plane changes are not self-merged.

Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
Signed-off-by: huangruiteng <14976749+huangruiteng@users.noreply.github.com>
@huangruiteng
huangruiteng force-pushed the codex/event-completion-transaction-0924 branch from 710b262 to 92ba6b1 Compare September 25, 2026 03:47

@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 92ba6b11d8c76aefd9d64b8a58aaea1dbedcdf8a, rebased onto origin/main 1882feb0c1c0d13b0b3149b8d68404a7954f63d0. The rebase touched one file: loopx/control_plane/effect_runtime_handlers.ts now keeps the upstream withCoordinationSourceTransfer wrappers for the two source-carrying operations and registers goal.state_event.plan_append unwrapped, because the Python caller sends an inline bounded identity payload rather than a source artifact.

动机

已复核基线行为:event 承载的 Todo 完成原先先写后继 add/claim 事件,再写父事件本身;编码阶段一旦报错,就留下一个「可运行但没有父完成事件」的孤儿子项。第二个反例是完成上下文读取之后源文件被改写,旧实现仍返回成功。集成 head 上这两个反例分别被拒绝或产出连贯状态,而它们在基线 1882feb0c 上失败。

对长期推进而言,这个修复让「完成一个 Todo 并原子发布其后继」成为一次可重放的持久动作:修复前一次失败会同时污染后继与父事件,恢复只能靠人工判断;修复后整个批次在事件锁内比较源校验和,并用一次原子替换发布,重放只确认持久性而不新增后继。对用户旅程而言,受影响入口是 loopx todo complete(事件投影 Goal 的 legacy 适配路径)与消费事件流的读侧:成功仍然返回空列表字段,重复调用返回幂等结果,源在读取后变化时返回可操作的源漂移失败而不是静默成功。

改动思路

新 owner 是 TypeScript 的 goal.state_event.plan_append:Python 在持有文件锁时把「已存事件身份 + 请求批次身份 + 源校验和 + 期望校验和 + 当前最大序号」交给 TS,TS 一次性决定整批的 append/replay 与序号分配,并对 expected_checksum 不匹配返回 event_source_changed、对同 id 不同指纹返回 event_id_conflict。Python 保留事件编解码、锁、字节与文件系统副作用,并新增 StateEventSourceChangedError 与 StateEventCommitUnknownError,把「源变了」和「可能已落盘」区分开。后继派生也改为 consumer 角色:Python 不再自己决定 continuation policy、capabilities、repository、excluded agents 与绑定默认值,而是编码 TS 已给出的 typed proposal。

语义与集成

append_many(events, expected_checksum=...) 只接受 list/tuple 的 eager 批次;lazy iterable 仍走每条可见的旧路径,并在带 expected_checksum 时直接拒绝,避免「部分可见」被误当成原子批次。历史 JSONL 字节(包括无害空行)被完整保留,重放路径在文件存在时额外做一次持久性验证,因此「上一次替换已成功但目录 fsync 失败」不会被当成未写入。RFC ledger 与两本双语 RFC 页只补齐了当前实现清单与 5–8 包的估算口径,没有把计划写成承诺。

具体改动

  • loopx/control_plane/goals/state_event_append.ts:新增 planStateEventAppend,在纯函数内完成整批冲突/重放判定与序号分配,越界或非法身份直接抛 EffectRuntimeRequestError。
  • loopx/control_plane/effect_runtime_handlers.ts:注册该 handler;按上游约定只给真正带源传输的操作加 withCoordinationSourceTransfer。
  • loopx/event_sourced_state.py:append_many 增加 expected_checksum 与原子发布;新增 StateEventSourceChangedError、StateEventCommitUnknownError;重放时校验持久性。
  • loopx/control_plane/todos/event_writeback.py:_append_event_projected_successor 改为 _encode_event_projected_successor,只编码 TS proposal 并返回事件批次;完成事件与后继事件合并成一次 append_many;replay 分支先验证源校验和,漂移则返回既有的完成校验失败结构。
  • 测试与证据:tests/control_plane/test_event_todo_transaction.py(原子发布、源漂移、重放)、tests/test_event_sourced_state_store.py(eager/lazy、字节保留、提交未知)、tests/control_plane_ts/state_event_append.test.ts(6 例,含原型键名)、examples/control_plane/event-completion-rehearsal.py(真实源、临时 registry 与事件日志的完成+后继+重放演练),以及双语 ledger 与 test_todo_mutation_authority.py 的 pair 调整。

对主干的风险

最大风险是发布方式从追加变成整文件原子替换:读者必须重新打开文件,且单次完成要重写整份日志。我在真实 6.1 MB 源上的一次 CLI 完成耗时约 2.4 s,作者测得同一份 707 KB 日志的七组热批中位数从 11.33 ms 升到 28.94 ms;这是可解释的持久性代价,而该适配器不是未来的高吞吐 provider,批次大小由调用方决定。第二个风险是跨语言传输面:RPC 只带身份与校验和,不带正文,因此 3 MB 级证据仍可在 4 KiB 级事实内传递,但也意味着任何身份字段的错误都会表现为拒绝而非静默。第三个风险是目标级门禁当前为红:canary premerge --goal-id loopx-meta 选中 19 项检查,其中语义词汇漂移 smoke 因模块对预算 44 > 43 失败,我在干净的 origin/main 1882feb0c 上复现了同一失败,故它属于继承的基线红灯而非本 diff 引入;本目标的评审策略不等待远端 CI,PostgreSQL 与打包前端不在本切片内。

我的整体评价

没有发现阻断项。这个增量把一个真实的可运行孤儿与「读到旧上下文仍返回成功」修成了类型化、可重放且带源见证的原子批次,并且顺带删掉了 Python 侧与 TS 重复的后继默认值/归一化决策,符合仓库「不要第二个决策 owner」的要求。验证覆盖了正例、负例与真实后端:128 个 focused Python 用例、3076 个 control-plane TS 用例(0 失败)、严格 mypy 23 文件、ruff、类型检查与真实源演练全部通过;变更质量回执 cqr_78837c25c6ea6f655fbc 对该精确指纹有效。需要维护者知道的两点:继承的词汇预算红灯需要单独决策,以及该 legacy 事件写路径应随其最后一个调用方退役。建议按维护者流程合并。

English verdict: APPROVE - exact head 92ba6b11d8c76aefd9d64b8a58aaea1dbedcdf8a; the atomic, source-bound event batch plus the TS-owned planner removes a real orphan/success-after-change risk and de-duplicates Python successor decisions, with focused Python, full control-plane TypeScript, strict typing and a real-source rehearsal green. The only gate failure is the inherited module-pair vocabulary budget, reproduced unchanged on a clean origin/main checkout.

@huangruiteng
huangruiteng merged commit bd24092 into main Sep 25, 2026
20 of 25 checks passed
@huangruiteng
huangruiteng deleted the codex/event-completion-transaction-0924 branch September 25, 2026 04:04
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Merge note — #5003 merged on the reviewed exact head

Merged via admin bypass as bd24092af976c914fff74dcf818d9f4a72f6aed2; all 15 changed paths in the merge commit are byte-identical to the reviewed head 92ba6b11d8c76aefd9d64b8a58aaea1dbedcdf8a, which was rebased onto origin/main 1882feb0c before review.

Changed surfaces: the new TypeScript event-append planner and its handler registration, AppendOnlyStateEventStore.append_many with compare-and-append and atomic publication, the event-owned Todo completion adapter (successors now encoded from TS proposals), the bilingual RFC ledger/checkpoints, three test suites and one public-safe rehearsal example.

Rebase repair: one conflict in loopx/control_plane/effect_runtime_handlers.ts. Upstream's withCoordinationSourceTransfer wrappers are kept for the two source-carrying operations, and goal.state_event.plan_append is registered unwrapped because its Python caller sends a bounded inline identity payload rather than a source artifact. A leftover conflict marker from the first resolution attempt was removed and the change folded into the original commit before pushing.

Checks on the reviewed head

  • pytest over the event-transaction, Todo-mutation-authority, event-store and local-coordination suites — 128 passed;
  • npm run test:control-plane — 3076 tests, 3047 passed, 0 failed, 29 skipped;
  • npx tsc --project tsconfig.control-plane.json --noEmit clean; uv run --extra test mypy clean (23 files); ruff check on changed paths clean; git diff --check clean;
  • real-path rehearsal examples/control_plane/event-completion-rehearsal.py against a detached 6.1 MB source with 949 backfilled events: one synthetic completion plus successor published atomically, replay_unchanged, source digest unchanged;
  • change-quality receipt cqr_78837c25c6ea6f655fbc for scope fingerprint 78837c25c6ea6f655fbc4e34f8d6d324b047674eaedf70c47d89b0b3fa69bc39 (verified valid);
  • loopx pr-review --check-merge-readiness 5003@92ba6b11d --goal-id loopx-meta returned ready=true immediately before merge.

Failures, skips and holds: the goal-scoped premerge gate selected 19 checks; 18 passed. examples/semantic-vocabulary-drift-smoke.py fails on the module-pair budget (44 > 43) because of a pair merged earlier on main; the identical failure reproduces on a clean origin/main 1882feb0c checkout and the failure text does not mention any changed file, so it is inherited and not attributable to this diff. Remote CI was not awaited (wait_for_ci=false for this goal). PostgreSQL and the packaged frontend are untouched by this slice. Manual holds: none.

Residual risk: atomic publication rewrites the whole event log, so a reader holding an open handle must reopen; the rehearsal measured about 2.4 s for one completion on a 6.1 MB source. This legacy adapter should retire with its last caller, and event-writer/outbox binding remains open.

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.

1 participant