fix: retain archive receipts and safely validate scheduler transport - #4101
Conversation
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
|
Validation receipt for 5165626, baseline 488b9d4:
Economics: archive selection/CAS/receipt ownership remains TS; 0 deleted Python semantic LOC, 258 gross bridge/correlation LOC, product +287/−10 = +277 LOC. Changed archive crossings 3→4; crash before projection uses 2 and recovery 4. Two bounded pending slots per goal. Delete the Python ACK seam with the terminal facade when the native Todo CLI/projection consumer replaces it; keep independent crash/retry and provider conformance tests. Quiet 16 interleaved pairs: baseline/candidate p50 2398.53/2600.33 ms, p95 3544.75/3240.27 ms (p95 −8.59%, median +8.41%). Earlier sampling overlapping tests/build had p95 +84.29%, prompting the quiet rerun. Small noisy samples do not establish a general speedup; the extra durable ACK cost is explicit and available for maintainer review. Future-facing review covered old-v0 receipt identity, stale ACK exclusion, maintenance locking, and temporarily invisible committed receipts. Recovery ends at successful projection ACK; a response lost afterward is outside this contract. CI and normal maintainer approval remain pending; no admin bypass. |
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
|
Updated this same PR to 6977406, merging The original archive product/test/config blobs remain byte-for-byte unchanged, including all five product hashes in the real PostgreSQL qualification receipt. The only contribution edit aligns the bilingual delivery-contract retirement condition with the newly merged RFC: retire the separate ACK bridge as the concrete journal/projection consumer takes ownership, retaining necessary Python adapters; full native CLI migration is not a universal prerequisite. Integrated-tree validation: 91 Python tests passed, covering archive crash/retry, terminal lifecycle, projection, new canonical status and split-root write fences. TypeScript typecheck, changed-document public-boundary scan, and All checks on the previous head passed; new-head CI is running. No reviewer feedback was present at this check. CODEOWNER approval and independent approval after the latest push are still required; no bypass. |
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
|
Synced main through #4097 and fixed the archive/recovery integration at 28354cc. The combined missing-display tests reproduced an entrypoint failure: archive rejected the absent Markdown before reaching promoted canonical authority. Archive now resolves the registered path without requiring the display to exist; registry/path validation remains, and an unpromoted archive still fails through the existing legacy check. Native archive selection, receipt identity, pending-attempt retention, and ACK conditions are unchanged. Validation:
The bilingual contract now also states that explicit |
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
…bution Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
|
CI diagnosis and source repair for the scheduler transport false positive. The failing run on Repair commit now builds a separate safety-validation view for exactly three real scheduler argv fields. It strictly decodes bounded Base64URL/zlib/JSON, and the unchanged scanner recursively inspects every decoded field plus all unrelated content. The actual actor packet/argv, generic scanner, scheduler protocol, native admission checks and archive path are unchanged. Duplicate keys, non-finite JSON, invalid encoding, truncated/trailing streams, decompression overflow and alias/path tricks are rejected. Six fixed public synthetic wire cases failed before this change and now pass for full ACK/failure and candidate ACK with both argument forms. They preserve compressed bytes independently of zlib encoder versions. Reproduce the fixed boundary with: python -m pytest -q tests/control_plane/test_model_behavior_qualification.pyCurrent local results at |
…y contribution Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
Signed-off-by: Steven <96154058+steven-kid@users.noreply.github.com>
…egration Signed-off-by: huangruiteng <huangrt01@163.com>
…egration Signed-off-by: huangruiteng <huangrt01@163.com>
|
Integrated current main, including #4095, into the original development branch at Fresh integration evidence:
The prior CI shadow-recovery failure did not reproduce in the local suite. It is not being declared resolved by inference: fresh hosted CI is running at this exact integrated head, and merge remains held until its required checks are reconciled. No live Goal or production database was used; local runtime is Node 25.5, while hosted minimum/primary/forward versions remain separately reported by CI. |
huangruiteng
left a comment
There was a problem hiding this comment.
动机
复审覆盖整个 PR,绑定整合 head afbd777d33fe2badb32ba916e5a07ea93279e726。代码与本地真实路径验证未发现剩余阻塞项;合并仍需确认本 head 的 required CI。
这个问题不只是 Markdown 显示延迟:canonical archive 已成功,进程却在交付显示之前退出;随后公共 CLI 用新 head 派生新的 operation ID,导致重试丢失原结果、报告 no_change/count=0。已有 receipt 并不等于调用者知道该查哪个 receipt。我用同一个真实 CLI harness 在基线 23e650235 和本 head 上重现:基线返回零,新版找回原 receipt、moved IDs 和 count=1,canonical revision 不再推进。这是明确且值得交付的 D1 正确性增量。
改动思路
最小合理方案是延伸现有 native archive owner,而不是做新的通用 workflow engine。TS 在 commit 前持久记录 goal/role 的待交付操作关联:operation ID、保留数量、观察到的 revision 和 store identity。该记录不复制 Todo、选择结果或 receipt,也不要求 Agent 额外维护声明;它由真实调用自动产生并在交付后自动退役。
archive selection、standing-decision retention、CAS、历史 receipt 仍由原 native transaction 决策。Python 读取输入、调用 transaction、复用永久 Markdown projection,成功后发 exact-operation ACK。每个 Goal 最多两个本地 slot,历史再长也不增长;但它是本地调用关联,不应扩张解释为跨主机 request ownership。
本轮已将最新 main(包含 #4095)整合到原开发分支,没有另开替代 PR、强推或改变 provider 默认值。#4095 的 fence 文案兼容修复也保留在整合结果中。
具体改动
相对当前 main 仍为 17 个文件、+1373/-14:新增主要集中在回归覆盖、170 行归档关联 owner、137 行 scheduler 安全验证视图,以及双语协议说明。整合的主干历史不计为本 PR 新增功能或删除收益。
关键代码讲解
executeLocalArchiveAttempt(local_archive_attempt.ts:78):已有 pending attempt 先恢复原操作关联,拒绝不同 store/retention;没有 pending 时先持久记录,再交给原 transaction。结果不明确时保留 slot,只有明确无提交/空归档等情况才清除。executeCoordinationTodoArchiveCompleted:历史 replay 在当前 head 检查前执行;新请求可绑定实际观察到的 revision,漂移即拒绝。旧 v0 省略 revision 的 hash 保持兼容。preview 不消耗、重放或确认 pending operation。acknowledgeLocalArchiveAttempt(同文件:139):精确校验 operation、store identity、receipt schema 和 request hash 后写 null tombstone;过期 ACK 不能清除新 slot,更不能修改 canonical Todo。archive_canonical_todos_if_promoted(provider_terminal_lifecycle.py:502):只在非预览、确有归档且投影 delivered/current 时 ACK;ACK 失败单独报告 pending,不能把已提交的归档改报失败。未 promotion 的 Markdown 缺失仍拒绝,不借此建立隐式 canonical fallback。scheduler_transport_validation_view(model_behavior_packet_safety.py:108):只解码真实结构路径中的 typed scheduler transport,生成独立扫描视图;原 argv/packet 不变。它不是 scheduler admission。现有安全扫描器继续检查解码后的全部字段及扩展;duplicate key、非规范编码、尾随流、解压上限、错误 schema、别名和伪造 dotted path 均有负例。
对主干的风险
最强失败场景是“旧尝试永远占住后续批次”或“提交状态未知时误清理尝试”。真实 CLI 的进程退出、投影写失败、ACK transport 失败、缺失显示、后续新批次等测试,配合 TS 的并发同意图、stale ACK、head 漂移和 receipt 暂不可见案例,覆盖了这一边界。不能只凭 provider-level green tests 下结论,因此补做了公共 CLI 的基线/head 对照与独立 readback。
整合 head 的本地验证:
- 154 项 Python 测试通过,覆盖 archive crash/retry、packet safety、shadow CLI、canonical authority、governance 和 Monitor。
- 979 项 TS 测试通过,零失败/跳过,包含 File、NoKV 测试适配器,以及隔离真实 PostgreSQL 16.15。NoKV 测试适配器不等于 live NoKV 部署验证。
- 普通归档后的再次调用、Todo 的状态/claim/text、未 promotion 缺失文件拒绝在基线/head 保持一致;提交后崩溃的 receipt/count 修复是有意语义变化。
- 六个固定 scheduler wire 正例在基线实际失败、head 通过;私有字段和 malformed/overflow 负例继续拒绝,packet 字节保持不变。
- TS typecheck、项目支持的 Ruff、diff/public-boundary 检查通过。
本地 Node 为 25.5;托管 CI 的 minimum/primary/forward runtime 是独立证据,不混用版本声明。旧 head 的一次 shadow capture CI 失败在本地未复现,不能宣称已经找出该失败的根因;最终是否合并以整合 head 的 required checks 为准。
恢复保证止于投影 ACK,而非终端收到 stdout。ACK 后丢失 stdout 不在此次保证内;跨主机关联、elapsed soak、完整 provider cutover 也不在范围内。投影修复不是业务重新执行;单独 project-markdown 不确认 pending archive,协议已明确写出。
我的整体评价
APPROVE,合并前保留 required CI 条件。该 PR 的实现与真实问题相称,没有把 projection 重新变成 authority,也没有让 Agent 维护另一份 planning state。其新增持久关联无法只靠当前 head 推导,属于有必要保存的调用意图。
Future-facing pass:复用 transaction、receipt、maintenance lock 和 renderer 已经是恰当收拢;目前不应为了删除一个 ACK RPC 引入大规模 daemon/journal 重写。等既有 journal consumer 同时拥有投影交付和 exact-attempt ACK,再删除 Python ACK 桥接;保留永久 Markdown renderer。此次无须再扩展生产结构。
English verdict: APPROVE at afbd777d33fe2badb32ba916e5a07ea93279e726, with required final-head CI remaining a merge condition. Real CLI baseline/head comparison reproduces the attribution defect and verifies original-receipt recovery without another commit. Normal and unpromoted paths remain compatible. Integrated validation passed 154 Python tests and 979 TS tests, including real isolated PostgreSQL 16.15, plus typecheck/lint and six failing-before/passing-after scheduler-wire regressions. The local attempt is bounded retry correlation, not another Todo authority or a cross-host/exactly-once stdout guarantee.
|
Post-merge CI follow-up: the failing The merged head Prepared fix: treat a genuinely empty transaction as containing no guarded ref update. Keep phase validation, malformed nonempty-row rejection, provider integrity checks, and previous-hook delegation. A previous hook's rejection still propagates; new unreferenced commits remain blocked. No time-window, provider-default, promotion or Todo authority change is involved. Validation: the two new empty-transaction tests fail before the fix and pass afterward; all 23 change-window tests pass with native Git. The real Git 2.55 fetch/pull plus local-commit-denial path also passes after the fix. Test helper failures now show the synthetic command output rather than hiding it behind CalledProcessError. The PR was merged concurrently before this fix was pushed, so the fix is not included in merge commit |
|
The post-merge Git 2.55 compatibility follow-up is complete in #4171 and merged as The repository change-window hook now treats a genuinely empty reference transaction as containing no guarded ref update, while preserving phase validation, malformed nonempty-row rejection, previous-hook failure propagation, provider integrity checks, and new-commit blocking. The real Git 2.55 fetch/pull regression and all 23 focused change-window tests passed; hosted CI, Ruff, diff, DCO, and public-boundary checks also passed. |
The public promoted
todo archive-completedcommand can commit an archive and then lose its result before Markdown projection. Its retry currently derives a new operation ID from the advanced head and returnsno_change/moved_count=0. This PR keeps the accepted attempt until projection delivery, so that retry returns the original receipt, count, IDs, and revision without another canonical commit.Addresses the reproduced finding in the post-merge review of #4053; related to accepted RFC #3225. This is one focused correctness follow-up to the existing native archive owner.
A recurring CI qualification failure is also repaired here: compressed public scheduler facts can accidentally match the credential heuristic before actor dispatch. A bounded validation copy decodes only the two typed scheduler hints in the full packet and the candidate packet's ACK arguments; the unchanged safety scanner checks every decoded field and all other values. The original packet/argv, scheduler protocol and archive transaction remain unchanged. Invalid encodings, duplicate keys, trailing streams and decompression overflow fail closed. This is qualification support for the same PR, not another archive feature or migration payoff.
Behavior
todo project-markdownrebuild does not ACK the attempt; the next matching archive replays and ACKs that prior batch.Recovery ends at projection acknowledgement. This does not promise recovery of stdout lost after ACK, or discovery of historical invocations made before attempt records existed. The bilingual delivery contract records that boundary.
Validation
main@23e6502359112ee992ba1f91284159a33f17a5cd.staticpassederasableSyntaxOnlygate on Node 24.21.0, three workflow-declaration smokes (GitHub Actions runtime, frontstage pages and full-public-smokes inventory), public-boundary scan andgit diff --check. These declaration smokes do not execute Node 22.6/26 or the full smoke inventory. The qualification helper is unchanged from its passing Ruff/strict-mypy evidence.unitpassedreal_entrypointpassedreal_entrypointpassedreal_backendpassedintegrationpassedreal_backendnot_runintegrationpassedb9bd01a419f917b5c59d80e76c0c8bf53cd3316e: CI34431442569 completed with 17 successful checks and three intentional deploy/release skips. The earlier repair head also passed CI34363091205.integrationrunningregression_paritypassedb9bd01a419f917b5c59d80e76c0c8bf53cd3316e, 409 Python checks and all six isolated scope/monitor mutation canaries passed; each mutant failed by assertion. Their source owners and fixtures are unchanged in this sync. This is historical revision-specific evidence, not a new mutation run.Coverage and gaps: this synchronization inherits Node 24 primary CI, minimum-runtime merge gating and concluded-review readback from main. These are not new archive features or deletion credit. Archive/provider owners and the qualification safety adapter are unchanged. Public archive recovery is separately rerun through an isolated daemon on the new primary runtime; broader projection/scope/monitor integration evidence retains its prior revision. The inherited review correction exposes no execution artifacts for inventory-only concluded heads unless the caller explicitly selects an exact head for a fresh audit. It does not grant approval or merge authority.
Two initial workflow tests failed because the local test environment lacked the already-declared xdist/split/coverage dependencies. After installing those test dependencies in the disposable verification environment, the entire selected Python suite was rerun. A subsequent process-level check found the same-source Effect daemon cache ignores the PATH runtime change; the seven archive cases were therefore rerun with an isolated temporary runtime directory and their Node 24 executable mapping verified. The general Python/smoke rows retain their actual Node 25 daemon boundary. Node 24.21.0 was obtained from the official distribution and checked against its published SHA-256; the default runtime/install was not changed. Independent maintainer approval, required hosted CI and live NoKV qualification remain open. No new packaging/performance result is claimed.
Historical regression evidence remains exact-commit: baseline
488b9d40e394efa10dbfab8fa8f9db169a9408cbreturnedno_changeafter archive commit/crash; candidate5165626e803d1ee2993187dcde34e7a5a5c2352areplayed original count 1. Six fixed public-wire cases failed before the safety repair and then passed; the four former CI portfolio failures separately passed in the previous 409-test integration and full hosted CI. Original wheel/sdist, installed-entrypoint and CLI-budget evidence remains bound to5165626e803d1ee2993187dcde34e7a5a5c2352a; performance below retains its original revisions.Fixture impact:
coordination_production_scale_v0is unchanged; existing role/status, standing-decision, lease, ordering, and archive-pressure dimensions cover selection. The new independent stale-revision and historical-replay assertions run in the shared provider conformance suite, including real File and PostgreSQL arms. At original candidate5165626e803d1ee2993187dcde34e7a5a5c2352a, the read-only three-arm rehearsal used one public-safe synthetic complex source: 464 Todos, 64 graph leases plus 160 retired lease inventory entries; 155 archived. Provider heads, legacy active semantics, per-role order, non-target preservation, and source fingerprints all agree. This synthetic rehearsal is not live production promotion evidence; no active goal/provider/registry was changed for tests.Scoped T0 / D1 handoff
This PR contributes the archive receipt-retention candidate to D1; it does not declare D1, local-profile qualification or whole-Goal cutover complete. The exact reconciled base and tested head are above. No SQLite candidate or unmerged implementation is a dependency; no legacy caller is retired.
todo archive-completedarchive_canonical_todos_if_promoteddelivers Markdown.acknowledgeLocalCoordinationTodoArchiveclears only the matching local attempt after delivery; no second business commit.todo project-markdown/ recoveryD1 evidence at this candidate is bounded:
b9bd01a419f917b5c59d80e76c0c8bf53cd3316e, the projection recovery tests cover missing/rebuilt display, stale authority, preservation of damaged documents and non-Todo sections, concurrent restoration and the required private validation declaration on the shared scale fixture. Lost independent Goal narrative cannot be reconstructed from Todo authority.todo project-markdownfollowed by the matching archive retry delivers and ACKs the existing result. Recovery ends at exact delivery ACK, not stdout reception.D1 qualification must still evaluate the actual merged head and preserve uncovered delivery/backend conditions as holds. Next action: finish current CI and independent maintainer review of this candidate; no promotion or broader D1 completion is requested.
Migration economics and review limits
Independent review found and fixed both the old-v0 receipt identity trap and the applied-but-temporarily-unreadable receipt case. Future-facing pass applied to upgrade compatibility, maintenance exclusion, exact ACK identity, and bounded storage. Private state, raw timings, database URLs, local paths, credentials, and generated package artifacts are excluded from this diff. Ordinary maintainer review and required CI remain necessary; no admin bypass is requested.