fix(metadata-protocol): never invent event_seq/version from a failed history read (#4867) - #4980
Merged
Merged
Conversation
…history read (#4867) `SysMetadataRepository.nextEventSeq()` and `nextItemVersion()` both folded EVERY read failure of `sys_metadata_history` into `return 1` — the shape #4825 just fixed on the legacy `DatabaseLoader` path, sitting unchanged on the canonical transactional one, and here on TWO numbers rather than one. With rows already in the table, one flaky read handed the next row `event_seq = 1` / `version = 1`: a collision with an existing row, written successfully, logged nowhere. `version` is the worse half — `nextItemVersion()` reads MAX from history precisely so a delete + recreate keeps incrementing instead of restarting at 1, so a read failure restored exactly the behaviour the method exists to prevent, while `MetadataManager.rollback(type, name, version)` and the rollback REST route resolve a snapshot BY that number. Being inside a transaction does not help: a transaction serialises concurrent writers, but a successfully committed transaction commits a wrong number just as durably. What it does give is the clean remedy — throw, and the whole write rolls back rather than committing an invented number. Now discriminated by error type, reusing #4825's discriminator rather than starting a second vocabulary: only a genuine missing table returns 1; every other read failure reports the consequence and the remedy once at `error` (AGENTS.md degradation log levels) and rethrows. `isMissingTableError()` was internal to `@objectstack/metadata`, so it is now exported deliberately through a new leaf subpath, `@objectstack/metadata/errors` — not the package root, whose entry would drag the manager, every loader and their deps behind a 40-line predicate, which is what would tempt the next author into copying it instead. Fixes #4867 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 7 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…metadata-repo-seq
…metadata-repo-seq
…-dispatch ledger `check:engine-double-contract` (#4550, landed today) flags the fake engine in `sys-metadata-repository.history-counters.test.ts`: its `delete` does not route through `assertEngineDeleteDispatch` from `@objectstack/objectql`. The gate's preferred remedy — add objectql as a devDependency — is not merely unreviewed here, it is CYCLIC. `@objectstack/objectql` already depends on `@objectstack/metadata-protocol` in `dependencies`, so the edge makes turbo refuse the graph outright; measured by adding it and reverting: Cyclic dependency detected: @objectstack/metadata-protocol#build, @objectstack/objectql#build So this takes the gate's other sanctioned route: a measured baseline entry naming the cycle as the reason, classified DEBT rather than EXEMPT because the ledger's own rule reserves EXEMPT for doubles nothing drives, and this one is driven (the #4867 delete-path test). The entry's `closes` names the only route that actually exists — sink the predicate into a package both sides already depend on — because the four sibling metadata-protocol entries prescribe the devDependency this commit just measured to be impossible. Filed separately rather than edited here: their text is not this PR's to rewrite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4867
问题
packages/metadata-protocol/src/sys-metadata-repository.ts的两个计数器各有一个同形的catch,把读sys_metadata_history的全部失败折成同一个答案:这是 #4825 刚在
DatabaseLoader(TSDoc 自称 legacy、非事务的那条路径,PR #4872)上修掉的形状,原样长在 canonical 路径上。而且这里是两个数字:event_seq—— 历史排序与 rollback 定位的依据。表里已有 N 行时,一次瞬时读失败(连接抖动、超时、权限)让下一条拿到1,与既有行撞号;version——nextItemVersion()的 TSDoc 明说它刻意从 history 取 MAX「so delete + recreate continues incrementing instead of restarting at 1」。一次读失败正好把它恢复成它明确要避免的那个行为;而MetadataManager.rollback(type, name, version)与POST /api/v1/meta/:type/:name/rollback正是按这个数字定位快照 —— 撞号之后回滚可能落到另一条记录的同号版本上。危害与 #4825 相同,是「落盘的字节是错的」而不是「字节没落盘」:insert 成功、日志一行没有、系统对外完全正常,重试不修、重启也不修。
「在事务里」并不能挡住它。 事务解决的是并发撞号;它对「从一次失败的读推导出来的数字」没有任何意见,一个成功提交的事务照样把错号提交得同样持久。事务真正给出的是干净的补救:抛出去,整笔写入回滚,而不是提交一个编造的号。
改动
按错误类型判别,复用 #4825 的判别器,不另起一套:
1确实是下一个号,静默返回,fresh DB 照常启动;error上报后果(写入已被中止、事务回滚、什么都没提交;若按旧行为发1会与既有行撞号,使版本顺序不可信、回滚目标可能指向另一条记录的同号版本)与修复动作(修数据源/驱动错误后重试),然后原样抛出让事务回滚。一次故障只说一次,恢复时补一条info。判别逻辑集中在新的私有
historyCounterVerdict(),两个方法共用一个判别、一个报告开关。@objectstack/metadata/errors(新增叶子子路径导出)isMissingTableError()此前是@objectstack/metadata的内部工具,消费者在另一个包。依赖图已核:@objectstack/metadata的依赖闭包(core / metadata-core / metadata-fs / platform-objects / spec / types)不含metadata-protocol,无环。选「从现有归属地显式导出」:在
metadata-protocol里复制一份会重建 #4825 刚消灭的双源问题(同一个问题两套「哪些驱动错误算良性」的词汇表,谁先学会一个驱动怪癖谁就先漂移);下沉到公共依赖本轮不可行(packages/spec冻结、packages/types有并行改动),且本次导出不妨碍之后再下沉。导出的是叶子子路径而非包入口:根入口会拖进 manager、全部 loader 与其 YAML/文件系统依赖,只为一个 40 行谓词付这个重量,正是把下一个作者推回「复制一份」的原因。仅导出
isMissingTableError;同族的isSchemaAlreadyExistsError包外无消费者,保持内部。测试
新增
packages/metadata-protocol/src/sys-metadata-repository.history-counters.test.ts(11 例)。测试用的 engine fake 实现了真实事务语义(txn body 抛出则什么都不提交)—— 这是承重的,不是装饰:命题正是「事务中止而不是提交一个错号」,而忽略回滚的 fake 分不出中止与提交。sys_metadata行也一并回滚;error一行同时点名后果(COLLIDES/= 1/ ordering untrustworthy / rollback)、已发生的事(ABORTED/ rolled back)与修复动作,并携带原始驱动错误;info,且恢复后从既有 MAX 续号(version 3 / event_seq 4),不从 1 重启;vi.mock证明消费的就是导出的那一个函数(翻转它的判决会翻转仓库行为),外加源码级断言 —— 本文件不得出现第二套驱动错误词汇表。变异验证(把
catch改回等价的「一律 return 1」):11 例中 7 例转红,其中 4 例良性方向仍绿 —— 两个方向都被钉住。两包无
typecheck脚本(均在 #4311 DEBT 台账内),类型面由 tsup 的 DTS build 覆盖:turbo run build两包均绿。另跑check:published-files、check:durability-log-level、check:type-check-coverage、check:startup-registry-verdict、check:release-notes,以及改动文件的 eslint —— 全绿。范围
packages/spec/**与生成物:零改动。packages/metadata-protocol/src/protocol.ts:零改动。content/docs/releases/:未触碰;user-visible 部分走 changeset。同文件其余
catch已复查:close()与broadcast()的两处是 watcher 回调隔离(功能性降级,正确);publishDraft()的 draft-drain 一处是同一族「一个良性原因赦免了所有原因」的形状,但不属于本 issue 的编号家族,且「抛 vs 只报」需要单独判断(它发生在put提交之后),按 Prime Directive #10 记为 #4981,未在本 PR 修改。🤖 Generated with Claude Code
https://claude.ai/code/session_01NrmBxj8rK2uGCnh9aipjwX