Skip to content

refactor(authority): unify retained journals and enforce scan snapshots - #4287

Merged
huangruiteng merged 2 commits into
mainfrom
codex/authority-journal-read-contract
Sep 13, 2026
Merged

huangruiteng merged 2 commits into
mainfrom
codex/authority-journal-read-contract

Conversation

@huangruiteng

Copy link
Copy Markdown
Collaborator

Authority journal readers could acknowledge a positive checkpoint against an empty store, and PostgreSQL scans could mix an older head with concurrently committed rows or silently skip missing history. This refactor gives all four providers one scan contract and gives File/NoKV one retained-journal decoder and append builder.

  • Reject non-string cursors before storage access and out-of-range checkpoints even on empty stores. Validate contiguous page coverage, lookahead, operation uniqueness within the page, and final-head agreement.
  • Use one repeatable-read snapshot for PostgreSQL reads. Preserve provider-specific transactions, CAS, revision hashes, wire schemas and File/NoKV persisted bytes.
  • Extend shared complex-fixture conformance, real PostgreSQL concurrency/corruption tests, managed-runtime readback, the isolated three-arm rehearsal, and both RFCs in English/Chinese.

Validation: final TypeScript typecheck and 1,312 tests passed without failures/skips, including isolated PostgreSQL 16.15; 73 Python CLI/projection/runtime tests passed. Four-provider baseline/candidate comparisons preserve legal six-commit histories, receipts and pagination; File/NoKV byte digests match. An owner-authorized read-only snapshot (474 Todos, 59 leases) preserves archive selection/order and exact File/PostgreSQL journal pages in disposable copies. Source state is unchanged.

Scope: 18 files, product code +200/-319 (net -119), no new RPC, bridge, capability or provider. The related simplification pass removes the callerless clone helper. Existing capability editor descriptors, configuration and user-visible schemas are unchanged, so no frontend companion change is required; affected CLI status/drain/projection interactions are covered. This qualifies the requested scan interval, not arbitrary historical corruption, retention/compaction, service readiness or Goal promotion. No default-provider change. Independent of open #4286; base is main at bfd1ec8.

Exact-source premerge passed: 4 direct checks plus 17 selected checks, zero failures/warnings/manual holds. Change-quality receipt cqr_f0971e17e0b38e1e9267 verifies the final diff. An initial installed-wrapper run selected another checkout and is excluded from candidate evidence; the passing run explicitly uses this worktree's source.

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

Request changes conclusion (author-owned PR; GitHub blocks formal self-review)

English verdict: REQUEST_CHANGES — exact head 4287@1588949ccc4c7ff5cf96250db9fc25807db91925. The required Node 22.6 minimum-runtime job cannot parse the new scanner and the merge-gate is failing.

动机

PR #4287 试图统一 File、NoKV、SQLite、PostgreSQL 四个 authority provider 的 retained-journal scan 解码与分页规则,补上 limit+1 lookahead、连续 cursor、operation identity 和 head lineage 校验,并让 PostgreSQL 的 head 与 rows 来自同一个 repeatable-read snapshot。基线中这些规则分散在 provider 内,腐败数据和分页边界需要多处同步修复。此 PR 保留每个 provider 的真实存储/事务边界,只抽出 provider-neutral 的 retained decoder 和 page validator;没有新增 RPC、capability 或写权限。

改动思路

AuthorityStore.scanCommitted 仍是唯一入口,caller 提供 after cursor 和 limit;provider 负责打开一次文件/NoKV/SQLite 读取或 PostgreSQL read-only transaction,构造同一快照的 head 与最多 limit+1 行;AuthorityJournalScan 负责跨 provider 的 range、精确行数、cursor 连续性、operation 唯一性和最终 head bytes/revision 匹配。decodeRetainedAuthorityJournal 负责 File/NoKV 的完整 revision/cursor/head chain。next_cursor 和 has_more 是由 head 与已验证 lookahead 派生的 projection,不是人工维护的 state。PostgreSQL 的 read-only path 改为 REPEATABLE READ READ ONLY,以避免 head/rows 跨 snapshot。

具体改动

本头涉及 18 个文件:新增 authority_journal_scan.ts(64 行),把 File/NoKV 的 retained decoder/append 逻辑集中到 authority_store_transactions.ts,四个 provider 的 scanCommitted 改为窄适配,PostgreSQL read transaction 使用 repeatable-read,更新 codec、conformance/transaction/PG 测试、Python readback 测试和中英文 RFC。

关键代码讲解

  • loopx/control_plane/coordination/authority_journal_scan.ts:16 的 prepare 统一解析 cursor 和 positive safe limit,并在输入非法时返回 invalid_scan_request。
  • loopx/control_plane/coordination/authority_journal_scan.ts:37 的 page 要求 rows 精确覆盖 retained interval,逐行验证 cursor offset、唯一 operation,并验证落在 head cursor 的 transaction 与 head projection/revision 一致。
  • loopx/control_plane/coordination/authority_store_transactions.ts:53 的 decodeRetainedAuthorityJournal 继续验证 File/NoKV 的全量 revision chain、cursor length 和末项 head lineage,provider revision 算法通过 callback 保持 provider-owned。
  • loopx/control_plane/coordination/postgresql_authority_store.ts:319 的 beginTenantTransaction 将 read-only 事务提升为 repeatable-read;scanCommitted 再在同事务读取 head、查询 limit+1 rows,并交给 common page validator。
  • loopx/control_plane/coordination/file_authority_store.ts:355 的 provider adapter 只读取单一 document、做 range check、传入 lookahead;NoKV/SQLite 采用同一适配模式。

对主干的风险

P1 blocker:GitHub Actions run 34702796801 的 node-minimum-compatibility 使用 Node.js v22.6.0,加载 loopx/control_plane/coordination/authority_journal_scan.ts 时在第 12 行 private constructor(after: string | null, offset: bigint, limit: number) 处报 SyntaxError: Unexpected identifier 'constructor'。这是 Node 22.6 的 TypeScript strip-types 解析限制,导致 37 个最低版本测试文件/子测试失败,随后 merge-gate 也失败;因此不是运行时数据边界的可选告警,而是 shipped minimum runtime 的导入级回归。请把参数属性改为 Node 22.6 可解析的显式字段/赋值(或等价兼容写法),然后在新精确 head 上重跑 node-minimum 与 merge-gate,并保留一个直接导入该模块的最低版本 smoke。

此外,本地 Node 25.5 的 typecheck、完整 TS 1265 tests(1264 pass、1 skip)和聚焦 scan/decoder 23 tests 通过,Python readback 3 tests 通过;但本地没有 LOOPX_TEST_POSTGRES_URL,PostgreSQL integration 只能记录为 skip,不能替代真实 PG snapshot 证据。分页/authority 语义和 default-off isolation 未发现额外问题,新增状态均为由 retained journal/head 推导的 typed projection。

我的整体评价

四 provider 的 owner placement 和分页不变量集中方向合理,改动量与 demonstrated consistency problem 相称;但当前 exact head 在声明支持的 Node 22.6 上无法解析,导致 required compatibility gate 失败。这里应先修复语法并在最低版本与 merge-gate 全绿后再判断 PG snapshot 的完整证据;当前结论是 REQUEST_CHANGES。

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/authority-journal-read-contract branch from 1588949 to c1f513a Compare September 13, 2026 05:53
@huangruiteng

Copy link
Copy Markdown
Collaborator Author

Self-refinement for exact head c1f513a44163c7158d1c1d04bf10fbbb71e1953a.

The Node 22.6 blocker from the prior review is resolved by rebasing onto current main@970159de4227f1855f3de5f1b09c430501875ff7: the repository's public minimum is now Node.js 22.18.0, and the minimum-runtime workflow uses the same version. The scan module's constructor is already explicit field/assignment code; no compatibility shim or production semantic change was needed.

Validation:

  • Node.js 22.18.0 full TypeScript control-plane suite: 1,289 passed, 1 skipped;
  • repository Node test suite: 1,289 passed, 1 skipped; typecheck and mypy passed;
  • isolated PostgreSQL 16.15 authority-store integration: 53 passed;
  • Python authority journal readback: 3 passed; ruff and diff checks passed;
  • loopx canary premerge --from-git-diff: 17 selected checks, 0 failures, 0 holds;
  • exact-scope quality receipt cqr_0527cb1b6ae534799074 is valid and passing;
  • remote blocking checks are green, including node-minimum, all test shards, pytest, and merge-gate. SonarCloud is non-blocking and still pending.

No post-rebase review blocker remains. Proceeding with the authorized admin self-merge.

@huangruiteng
huangruiteng merged commit 34710d1 into main Sep 13, 2026
28 checks passed
@huangruiteng
huangruiteng deleted the codex/authority-journal-read-contract branch September 13, 2026 06:07
@wchwawa

wchwawa commented Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Post-merge LoopX-side evidence from the NoKV plane, as agreed in #4069. The review was bound to exact head 1588949cc; the merged commits (d6a41417e, c1f513a44) are byte-identical to the reviewed ones per git range-diff, and the only base delta is two SQLite lines from main. Verdict for the record: no blocking finding; approve.

Invariants checked

  • NoKV persisted bytes and revisions are unchanged. Identical three-commit sequences written by pre-refactor(authority): unify retained journals and enforce scan snapshots #4287 main and by this head produce byte-identical NoKV envelopes (sha256 5355785…, revision nokv:3:65e4f65…, generation 3) and, with a pinned identity, byte-identical File documents (bd93ad1…). Documents written by either version load, scan (limit 2, two pages, has_more then exhaustion), answer readReceipt, and accept an append from the other version; the appended envelopes are again byte-identical (c09c143… NoKV, 4bed0b2… File).
  • NoKV head/rows consistency holds by construction: readEnvelope returns one blob plus its generation, so the coverage and head-agreement checks see one snapshot. The storage-generation check still runs before the shared decoder and its reason strings are unchanged.
  • The empty-store correction has no production caller: all five TypeScript scanCommitted call sites pass null or the request cursor forwarded by the Python adapter, and no Python caller passes scan_after_cursor.
  • PostgreSQL's head === null snapshot only occurs for the uninitialized (0, 0, null) head row, so page([], null) cannot hide committed rows.

Validation actually run (Node 24.11.1, macOS)

Check Result
Live Stage 2A s2a.nokv_live_qualification against a real NoKV 0.11.0 shard owner (etcd 3.7.1 routing, S3-compatible object store, SDK 0.11.0 / API 1), on merged main 4d57f53af and on the reviewed head 13/13 checks passed on both: existing workbench identity, fresh target, create at generation 1, lost-response reconciliation, generation CAS, competing CAS with one winner and no double advance, independent transport readback, ambiguous/winner/loser receipts; final generation 3, cursor 3
Live Stage 0 s0.nokv_live_matrix (same stack) 13 NoKV rows and 12 parity rows true, restored_lineage_fails_closed true, on both trees
Complete ladder on merged main with the live stack 22 pass, 0 fail, 1 unverified (PostgreSQL, no local database), 1 pending (declared soak); 0 privacy violations
Validator mutants against the provider suites (control 169/0): disable coverage, cursor lineage, operation uniqueness, head agreement, range, has_more lookahead each fails at least one test (1, 1, 1, 1, 3, 6)
npm run test:control-plane on the reviewed head 1248 passed; the 2 failures (quota_monitor_poll_commit, monitor_successor, Node assert message type) reproduce identically on untouched main
Synthetic merge with main at review time conflict-free; tsc and 260 provider/shadow tests passed
pytest -m stage2c_e2e -n 4 --dist loadfile on the reviewed head 223 passed

Not run: the PostgreSQL integration test (no local database).

Non-blocking notes

  1. NoKV generation is derived two ways. appendRetainedAuthorityJournal passes the closure generation into providerRevision, while decodeRetainedAuthorityJournal passes Number(transaction.cursor). They agree only because the shared decoder validates entry.cursor === index + 1 before calling revisionFor and the NoKV wrapper checks storage_generation === committed.length. A comment on that ordering, or passing the validated ordinal as a third JournalRevision argument, would keep a later reorder of the decoder checks from silently changing NoKV revision inputs.
  2. The shared decoder hard-codes retain-all (cursor === committed.length, whole journal in one envelope). That matches today's File/NoKV envelopes and the RFC sentence that a page does not certify a future compacted/segmented format. When NoKV takes the bounded-head/receipt-index shape from section 7.2, RetainedAuthorityJournal will need a versioned successor rather than a relaxed check; worth naming in that milestone so it is not treated as a decoder bug.

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.

2 participants